Skip to content
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

Move object supports visual and operator-pending modes #329

Merged
merged 2 commits into from
Dec 10, 2022

Conversation

kiyoon
Copy link
Collaborator

@kiyoon kiyoon commented Dec 8, 2022

This pull request resolves #6, to make the move operation consistent with vim's behaviour.
Now you can move in visual mode or operator-pending mode.
For example,

v]M will select until the end of the function, assuming the key binding following the README.
dv]M will delete until the end of the function.

@theHamsta
Copy link
Member

Thank you!

@theHamsta theHamsta enabled auto-merge (rebase) December 8, 2022 19:54
auto-merge was automatically disabled December 9, 2022 01:40

Head branch was pushed to by a user without write access

@kiyoon
Copy link
Collaborator Author

kiyoon commented Dec 9, 2022

Thanks for the approval. I noticed my change affected swap and lsp_interop keybindings as well, so I made another commit to fix it.

Now, move can be done in normal, visual, and operator-pending modes,
swap is only in normal mode,
lsp-interop maps key bindings in normal and visual modes.

@theHamsta theHamsta enabled auto-merge (rebase) December 10, 2022 09:15
@theHamsta theHamsta merged commit 37a810a into nvim-treesitter:master Dec 10, 2022
@theHamsta
Copy link
Member

@kiyoon you seem to be very active in improving this plugin. Would you be interested in helping with maintaining this repository?

@kiyoon
Copy link
Collaborator Author

kiyoon commented Dec 12, 2022

@kiyoon you seem to be very active in improving this plugin. Would you be interested in helping with maintaining this repository?

Thanks for the offer! I by no means have a full understanding of the project, but I'm happy to help and looking forward to further contributions :)

@theHamsta
Copy link
Member

At the moment, I only have little time to look at open PRs and I don't use the plugin often enough to feel confident about changes. It would be good when somebody who uses it more frequently would have a look which changes do make sense and which one

These are some ideas I wanted to implement for a

  1. port to the native nvim query API: avoid nvim-treesitter.query and use vim.treesitter.query instead. This makes our code slow and complicated. This involves a decision on what to do with #make-range!
  2. simplify the code by better more general abstractions. This will likely happen during 1) automatically. Since I didn't do the refactor for 1) I didn't want to change the legacy code but there were a lot of PRs that added extra option via special cases for each of the modules. It would be better to drop some of the features for a time that have separate code paths just for select with a special option but have something that can be shared for select/move/swap and a variety of options or even with upstream nvim-treesitter
  3. add tests: this would make it easier to review PRs without the fear that they break another combination of options

I think I will be opening a issues with a "call for maintainers". So we can together tackle the open tasks.

@kiyoon
Copy link
Collaborator Author

kiyoon commented Dec 12, 2022

The milestones sound good.

  1. I didn't know there was a native API but seems like the port should be needed. For stability maybe we can make another branch and finalise porting in parallel.
  2. I agree if select / move / swap functions are more general, it should be easy to use the functions and people can customise their own text objects, and make their own keybindings as well. For example, select can return the node and the range, and make another function that "performs" select with that function (I believe treesitter have update_selection() function.) Or, maybe adding some regular expression matching will enable better customisation.
  3. Never tried making tests on neovim, but I assume you are given a sample file and perform swap / move / select operations on many different languages and checking how it ends up should be implemented.

I'll check the call-for-maintainers issues whenever I have time.

@kiyoon
Copy link
Collaborator Author

kiyoon commented Dec 12, 2022

I have an idea for the tests.

We can pull many code from many open sourcr projects. We divide that into two categories: stable version from popular projects, and unstable any repositories.

We run many sets of treesitter select, swap, move operations. Manipulate the code with the selection (like replace the selection to something) and keep the diff of the files. Maybe we shouldn't keep this data in the git repo though.

For every commit, we perform the "consistency test", to ensure the manipulation behaviour is equal to the previous commit. If the test fails, we can investigate what changed. If we made a breaking change (like we fixed bug in movement) then we can just enforce the commit with updating the tests.

The second split of the data (random code, not popular repos) can introuce a lot of edge cases due to the code not structured or complete.

How does this sound? Since I don't work with every language, it's hard to write all tests so this way it should be a good starting point of the automated tests.

@kiyoon kiyoon deleted the feat-move-x-o-mode branch April 10, 2023 11:36
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.

Make move commands work in visual mode
2 participants