-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support counts like all native tag nav mappings do #22
base: master
Are you sure you want to change the base?
Conversation
let orig_height = &l:previewheight | ||
let &l:previewheight = a:height | ||
execute a:cmd | ||
let &l:previewheight = orig_height |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, this should maybe have the execute
in a try with the reset in its finally
. Though perhaps that's overly defensive when it's an internal function and we're reasonably certain it's called with safe operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When trying to jump to a non-existing tag, that seems to be reported as an error (E426). I'm not sure how that behaves wrt finally
. Maybe you can test with that to decide whether finally
is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, my intuition was coming from somewhere 😄
It turns out that the 'previewheight'
value was restored on error (because execution continued, :h except-compat
), but there is another reason to catch the error: it leaves behind a spurious preview window of the current buffer, as if you'd run :pedit
with no argument. Fixed.
I can squash these changes later if desired, just going to leave it until further review.
Generalizes the approach so that the function doesn't need logic to handle split, preview, etc. Also no longer results in the cursor being moved to beginning of the function call (or record/macro after vim-erlang#15) in the original buffer.
Vim's native window tag mappings (split- and preview-related) use count to affect the size of the new window. Regular mappings like `CTRL-]` use count to jump to a match index like `:tag` does. These changes mimic all of the above behavior accurately with this plugin's enhanced notion of tag identifiers for Erlang.
Vim's native window tag mappings (split- and preview-related) use count to affect the size of the new window. Regular mappings like
CTRL-]
use count to jump to a match index like:tag
does.These changes mimic all of the above behavior accurately with this plugin's enhanced notion of tag identifiers for Erlang.
This is based on my branch from #21 because it includes support for the preview window mappings, but since it's a more substantial change I thought it made sense to review this independently.