Skip to content

Conversation

@jebaum
Copy link
Contributor

@jebaum jebaum commented Mar 19, 2016

In addition to the workaround, I added a silent! to the MoveSelectionDown() function, as I couldn't see a good reason why it wasn't already there, and assumed it was an oversight. Let me know if it was left out for a reason.

@tpope
Copy link
Owner

tpope commented Mar 19, 2016

silent! is fine but but it definitely shouldn't apply to the fdm dance. In fact by doing so you've missed the fact you have a syntax error that completely breaks things.

@jebaum
Copy link
Contributor Author

jebaum commented Mar 20, 2016

Can you be more specific? I tested it locally and it works perfectly for me, if something is wrong I doubt it's a syntax error

@jebaum
Copy link
Contributor Author

jebaum commented Mar 20, 2016

I'm on mobile but it seems like a | is missing that definitely shouldn't be locally. Either that or for some weird reason it works anyway. Cant check until tonight

@tpope
Copy link
Owner

tpope commented Mar 20, 2016

Yes a | is missing. But point being there's no reason to embed all that inside a string eval to begin with.

@jebaum
Copy link
Contributor Author

jebaum commented Mar 21, 2016

I agree it's missing, but have you pulled this locally and tried it? The map works, although I honestly don't know why, I would expect it not to now that I've looked more carefully. I am not sure how you would like to see this implemented if not with a string eval, but I'm happy to change it, I just want to see this functionality merged asap

@jebaum
Copy link
Contributor Author

jebaum commented Mar 21, 2016

I see now, fdm isn't getting set back to the value that was saved, although I'm still surprised the move works at all. Fixed

@tpope
Copy link
Owner

tpope commented Mar 21, 2016

Take your additions and move them outside the quotes onto their own lines.

@jebaum
Copy link
Contributor Author

jebaum commented Mar 22, 2016

fixed. do you think it would be better to use window local fdm? setlocal, let &l:fdm ?

@jebaum
Copy link
Contributor Author

jebaum commented Mar 22, 2016

changed it to use local values, that seems much cleaner. tested it and it works fine. should be ready to merge now I think

@jebaum
Copy link
Contributor Author

jebaum commented Apr 11, 2016

ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants