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

vim: Support gn command and remap gn to gl #9982

Merged
merged 7 commits into from
Apr 6, 2024

Conversation

joaquin30
Copy link
Contributor

@joaquin30 joaquin30 commented Mar 30, 2024

Release Notes:

  • vim: Breaking Change. Updated gn and gN to select the next search result as in vim. Vim: support gn command #4273. Adding multi-cursors to the next/prev copy of the word under the cursor is now bound to gl/gL`.

@algora-pbc /claim #4273

This is a work-in-progress. The process for gn command is:

  • maintain updated vim.workspace_state.search.initial_query
  • modify editor.select_next_state with vim.workspace_state.search.initial_query
  • use editor.select_next()
  • merge selections
  • set editor.select_next_state to previous state

To make this possible, several private members and editor structures are made public. gN is not yet implemented and the cursor still does not jump to the next selection in the first use.

Maybe there is an better way to do this?

Copy link

cla-bot bot commented Mar 30, 2024

We require contributors to sign our Contributor License Agreement, and we don't have @joaquin30 on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@joaquin30
Copy link
Contributor Author

@cla-bot check

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Mar 30, 2024
Copy link

cla-bot bot commented Mar 30, 2024

The cla-bot has been summoned, and re-checked this pull request!

@joaquin30
Copy link
Contributor Author

joaquin30 commented Mar 30, 2024

Also the work of @Neptune650 in #9969 could serve for moving the cursor when vim is in normal mode.

Edit: I included some of the code of #9969 for searching matches and added basic operator support, dgn and ygn should work.

@ConradIrwin
Copy link
Member

@joaquin30 @Neptune650 I like that we are re-using the search bar for this. In non-vim mode the default behavior is to select the match, so (maybe?) we can simplify the normal mode version of this to something like:

editor.set_collapse_matches(false);
search_bar.select_match(Direction::Next, vim.take_count().unwrap_or(1), cx);
editor.set_collapse_matches(true);
// handle pending operator

I see you have code to handle expanding the selection if you're already in visual mode, n and N should be updated to match gn and gN in visual mode. It's not perfect with zed's multi cursors, but fixing that seems out of scope of this change.

If it'd be useful I'd happily pair with one or both of you to help get this over the line: https://calendly.com/conradirwin/pairing

@ConradIrwin ConradIrwin self-assigned this Apr 1, 2024
@baldwindavid
Copy link
Contributor

Great to see some of this gn behavior working its way in. I know this is wip and there is no cgn yet. One thing I can see from dgn is that, although it works for one instance of the match, it does not allow for n followed by . to repeat the operation for the next match. And, of course, that means just . (without n first) does not repeat the operation on the next match. My workflow all day long is typically:

  • search for partial words
  • cgn to change the partial match
  • any amount of .s to change instances of the match in the file

Or, sometimes I'll look through the matches with any amount of ns prior to . to make the change on a given instance. Ignore if this is already known/planned. Thanks!

@ConradIrwin ConradIrwin marked this pull request as ready for review April 5, 2024 22:21
@baldwindavid
Copy link
Contributor

baldwindavid commented Apr 6, 2024

cgn seems to be working which is amazing. I do notice that dgn followed by two .s freezes up the editor every time I've tried thus far and I need to force quit. I see this marked as ready for review so mentioning, but looks like some commits are still coming in so maybe a known issue.

@ConradIrwin
Copy link
Member

@baldwindavid nice catch! I hadn't caught that, but can reproduce, thank you for the heads up

@ConradIrwin ConradIrwin merged commit bf9b443 into zed-industries:main Apr 6, 2024
8 checks passed
@baldwindavid
Copy link
Contributor

baldwindavid commented Apr 6, 2024

Couple other things I'm seeing:

  • cgn and then holding . makes a number of changes (8-10 or so), but before catching them all stops changing selections and just keeps moving the cursor back one space
  • This might just be due to the sheer number of matching selections but doing ga after gn locks up the editor

@joaquin30
Copy link
Contributor Author

joaquin30 commented Apr 6, 2024

I was going to comment the same, but also using cw and pressing . 8-10 times has the same effect. And with dw it freezes the editor. Maybe this is another issue.

Edit: This is not related to the number of repetitions, but instead with how fast we are pressing ., If we press a little slowly it goes without a problem.

@ConradIrwin
Copy link
Member

cgn/cw and then . going backwards is interesting:

  1. For cgn I think in real vim if there is not another gn match, then insert mode is never entered/exited, so that doesn't happen. That should be easy to fix. @joaquin30 do you want to take a pass, or should I?
  2. For cw I think zed has a bug in w when the last character in a word is under the cursor, we seem to select one character past the end of the word. That seems like a different problem.

For ga after gn, I can't reproduce a freeze. ga is noticeably slow in a debug build on editor.rs in this repo after searching for fn, but in a release build that seems ok (with 434 cursors). Do you have more concrete reproduction steps?

@joaquin30
Copy link
Contributor Author

Yes, I will do it

@joaquin30
Copy link
Contributor Author

OK, the code is in #10237. There are two issues:

  • When there are no more matches, the next repetition still moves the cursor to the left. After that is the recording is cleaned.
  • If cgn is used when there are no matches, it cleans the previous recorded actions.

@baldwindavid
Copy link
Contributor

For ga after gn, I can't reproduce a freeze. ga is noticeably slow in a debug build on editor.rs in this repo after searching for fn, but in a release build that seems ok (with 434 cursors). Do you have more concrete reproduction steps?

@ConradIrwin - Okay, yeah, much better with a release build. I can get it to be sluggish searching editor.rs for the letters et, then ga after gn, changing to what, and then quickly attempting undo with u, but I'm mostly just stress-testing at that point.

@ConradIrwin
Copy link
Member

Makes sense, sorry I had misunderstood what was happening, and I think you're right that we need to stop repeating (though maybe if we wanted to emulate vim better we'd just ignore the next insert/normal switch, this seems fine for now).

Pushed a bit of tidy up to your branch, but this should be good to go.

ConradIrwin added a commit that referenced this pull request Apr 8, 2024
Release Notes:

- Fixed `cgn` backwards movement problem in #9982

There are two issues:

- When there are no more matches, the next repetition still moves the
cursor to the left. After that, the recording is cleared. For this I
simply move the cursor to the right, but it doesn't work when the cursor
is at the end of the line.
- If `cgn` is used when there are no matches, it cleans the previous
recorded actions. Maybe there should be a way to revert the recording.
This also happens when using `c` and `esc`

---------

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants