-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat(motion): allows movement to parent form's edges using (, )
#38
Conversation
(, )
(, )
Hey @Samy-33, thanks. Looks sick. Can you also update the readme with new keymaps? |
@armed, thanks for taking a look. Done. |
Thanks for taking the time to add this! Everything looks good code-wise. I'm just unsure about the behaviour of moving to the parent from a form edge. In this implementation it moves to the nodes direct parent and not the parent form: (1 @|(a))
=>
(1 |@(a)) I feel it makes more sense to move to the next parent form bracket. (1 @|(a))
=>
|(1 @(a)) |
I was also wondering if the motion should only be to brackets and was surprised when Treesitter would say that This behaviour only happens for shortcuts for a form, and in my opinion, it makes more sense to keep consider As |
This isn't quite accurate - Treesitter knowns nothing about Clojure and is not in itself considering the The way the Clojure TS grammar is structured the The same is true for other grammar components such as: `()
#{}
#_()
`#{} etc. I don't think in any of these cases it makes sense to treat the prefixes to these forms as forms themselves and we should instead be jumping to the nearest parent form (not just the treesitter AST parent).
These bindings don't exactly consider form prefixes as a form itself, but when using I do not think the same applies for navigating between brackets. I think the expected behaviour is more likely to be that these navigation functions move between form brackets only, ignoring node markers. |
FWIW I gave vim-sexp(-mappings-for-regular-people) a quick test to compare how it behaves and they also jump only between brackets, ignoring form markers like All we really need to change to support this is to wrap form_node_to_move_to = form_node_to_move_to:parent() in
|
You're right @julienvincent. I assumed that treesitter considered Thanks for pointing it out. |
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.
Cool this all looks good! Thanks for the contribution!
Yep, really useful addition, actually I'm still using vim-sexp with some mappings partially enabled. Parens one of them. |
closes #34