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.

Sign up for GitHub

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

Jump to bottom

vim mode: Pressing enter in the buffer search dialog moves to the second match #8049

Open
1 task done
hferreiro opened this issue Feb 20, 2024 · 15 comments · May be fixed by #25580
Open
1 task done

vim mode: Pressing enter in the buffer search dialog moves to the second match #8049

hferreiro opened this issue Feb 20, 2024 · 15 comments · May be fixed by #25580
Labels
vim

Comments

@hferreiro
Copy link
Contributor

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

  1. Search for a common string that appears multiple times on the current buffer.
  2. Press Enter.
  3. The cursor position is changed to the second match.

I'd expect that the cursor is positioned at the first match.

Environment

Zed: v0.122.2 (Zed)
OS: macOS 14.3.1
Memory: 32 GiB
Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

If you only need the most recent lines, you can run the zed: open log command palette action to see the last 1000.

No response
@hferreiro hferreiro added admin read bug [core label] labels Feb 20, 2024
@JosephTLyons JosephTLyons added vim and removed triage labels Feb 20, 2024
@ConradIrwin
Copy link
Member

To clarify reproduction steps here:

  • have vim mode enabled
  • use cmd-f to open search
  • search for something with multiple results
  • hit enter

(This doesn't happen if you use / to open search).

See also #7692

@hferreiro
Copy link
Contributor Author

It also happens when using /, but I have just found that in the first search it works as expected. Then, searching for the same string will trigger the bug. So it might have happened that you first tried with / and then with cmd-f?

@ConradIrwin
Copy link
Member

Hmm. I definitely did, thanks for clarifying.

I now can't reproduce this at all. Can you describe the reproduction steps in more detail?

@hferreiro
Copy link
Contributor Author

  1. press / to enter the search dialog after launching Zed
  2. write a word which appears multiple times in the buffer (e.g. "test")
  3. press enter. Now the cursor is at the first occurrence of "test"
  4. press /
  5. press enter. Now the cursor is at the second occurrence of "test"

@d1y
Copy link
Contributor

d1y commented Feb 21, 2024

I think it's the same problem: #7692

@ConradIrwin
Copy link
Member

@hferreiro hmm... The behavior you are describing is how it should work (each time you do /<enter> you should be taken to the next match).

I think I managed to reproduce what you're seeing using just vim mode though:

  • Open README.md
  • Type / o u enter (it should match the "You" on line 9)
  • Click on line 8
  • Type / o u enter again (re-typing the search).
  • It jumps to the "our" on line 11 instead of going back to the "You" on line 9.
Screen.Recording.2024-02-20.at.9.53.11.PM.mov

@hferreiro
Copy link
Contributor Author

Yes, but that's not the correct behavior. Enter should just unfocus and jump to the first result, as mentioned at #7692 (comment).

@hferreiro
Copy link
Contributor Author

Funny thing, at the forth step at #8049 (comment), if you don't type o u ( because the search string already appears in the dialog) and just press enter, it works as expected.

@github-actions github-actions bot mentioned this issue Feb 21, 2024
Open
@github-actions GitHub Actions
Copy link

Hi there! 👋
We're working to clean up our issue tracker by closing older issues that might not be relevant anymore. Are you able to reproduce this issue in the latest version of Zed? If so, please let us know by commenting on this issue and we will keep it open; otherwise, we'll close it in a week. Feel free to open a new issue if you're seeing this message after the issue has been closed.
Thanks for your help!

@github-actions github-actions bot added the stale Label used by `stale` action label Sep 24, 2024
@hferreiro
Copy link
Contributor Author

Still an issue.

@hferreiro hferreiro mentioned this issue Sep 25, 2024
Open
1 task
@github-actions github-actions bot removed the stale Label used by `stale` action label Oct 1, 2024
@hferreiro hferreiro mentioned this issue Jan 31, 2025
Open
1 task
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 4, 2025

Hi there! 👋
We're working to clean up our issue tracker by closing older issues that might not be relevant anymore. If you are able to reproduce this issue in the latest version of Zed, please let us know by commenting on this issue, and we will keep it open. If you can't reproduce it, feel free to close the issue yourself. Otherwise, we'll close it in 7 days.
Thanks for your help!

@github-actions github-actions bot added the stale Label used by `stale` action label Feb 4, 2025
@hferreiro
Copy link
Contributor Author

Still an issue.

@github-actions github-actions bot removed the stale Label used by `stale` action label Feb 11, 2025
@nilehmann nilehmann mentioned this issue Feb 16, 2025
Open
@nilehmann
Copy link
Contributor

nilehmann commented Feb 16, 2025 •
edited
Loading

The issue is caused by this if:

zed/crates/vim/src/normal/search.rs

Line 191 in 74d1a65

if (search_bar.query(cx) != self.search.initial_query)

That if skips the first match if the query hasn't changed, that's why this is only triggered when you search the same thing twice. Removing that line fixes the issue for me but some tests fail. I don't know exactly what the logic there is, but I think it's trying to emulate the behavior in vim when the current selection overlaps with a match, if this happens vim skips the match under the cursor and goes to the next. This is however not correctly replicated as evidenced by:

initial state: «ˇa»a aa
keystrokes: / a a enter
final state:

  • zed: «ˇaa» aa
  • vim: «aa ˇaa»

@ConradIrwin
Copy link
Member

👍 Nice find! I'd like to fix this to more correctly match vim, and it sounds like we need to maintain a little more or different state to correctly emulate how vim works here.

Happy to pair on a solution, or review PRs if you have an idea: https://cal.com/conradirwin/pairing

@nilehmann
Copy link
Contributor

I took a slot for Tuesday

@devzeth devzeth removed the bug [core label] label Feb 22, 2025
ConradIrwin added a commit that referenced this issue Feb 25, 2025
@ConradIrwin @nilehmann @Anthony-Eid
Fix search skipping in vim Mode
4085aa2 
Closes #8049

Co-authored-by: nilehmann <[email protected]>
Co-authored-by: Anthony Eid <[email protected]>
@ConradIrwin ConradIrwin linked a pull request Feb 25, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
vim
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix search skipping in vim mode zed-industries/zed
6 participants
@ConradIrwin @hferreiro @nilehmann @JosephTLyons @d1y @devzeth