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

Global search no longer adds into search history #11205

Closed
xJonathanLEI opened this issue Jul 17, 2024 · 8 comments · Fixed by #11209
Closed

Global search no longer adds into search history #11205

xJonathanLEI opened this issue Jul 17, 2024 · 8 comments · Fixed by #11209
Labels
C-bug Category: This is a bug

Comments

@xJonathanLEI
Copy link
Contributor

xJonathanLEI commented Jul 17, 2024

Summary

I'm firing this as bug report instead of enhancement as this used to work.

After upgrading to Picker v2, the global search feature turns from a command that accepts a pattern into a fully interactive picker. With the new picker-based search experience, the searched pattern is no longer persisted into the search history even when pressing Enter in the picker.

This breaks several usage patterns of global searching:

  1. I often find myself using the global search feature to find the files that contain the pattern. Once I grab the file and handle that occurrence, I typically just press n to jump to the next occurrence within the file, instead of heading back to the picker with <space> ', as doing this minimizes context switch. This used to work because the pattern used to be persisted when doing the global search in the first place - so pressing n works. It's no longer the case.

  2. I also find myself using the global search feature to replace or otherwise modify a specific string. One thing I like to do is to re-issue the same search again by doing the combo of <space> / <enter>. This is different from using the "last picker" as it performs a new search. By using the new search, I'm able to see my progress of replacing/modifying that string across different files. Once the new search becomes empty I'd know I'm done. But with the new global search experience it's no longer possible as it's not persisted. I have to enter the pattern each time for a new search.

Yes, I know a local search would still update the search history. So technically a workaround is to do a local search first, then trigger the global search... But that's not very nice at all.

Oh and there's another issue with the new picker-based global search experience: it only presents the latest search history item, and there seems to be no option to use any other older items like you used to be able to do. I think it's pretty common to search a recently searched pattern, but that's no longer doable. I'll need to submit another bug/enhancement for this though.

Reproduction Steps

Just global search anything and realize that it's no in the search history.

Helix log

N/A

Platform

Any really

Terminal Emulator

Any really

Installation Method

Any really

Helix Version

helix 24.7 (b0cf86d)

@woojiq
Copy link
Contributor

woojiq commented Jul 17, 2024

Agree, the global search history was soo good.

@pascalkuthe
Copy link
Member

pascalkuthe commented Jul 17, 2024

I had the same complaint so Mike added some basic history support to global search.

Global search appends to the history register when you select a picker entry so the usecasw of globalsearch and using n afterwards should still work.

Regarding selecting from the history you will notice that when the glboal search picker is empty it preview the last entry in the history and you can hit enter to use that.

I do wish we had a way to scroll history like we could with up/down. Mike different that to future work since that would take more complex ui

@the-mikedavis
Copy link
Member

Actually I think there's an oversight with saving to a register on Enter. The prompt is set up to do it but we don't pass the Enter event to the picker's prompt. Should be a small fix.

Selecting from the history register is basically just a UI problem as Pascal says. Figuring out where to show the options is tricky - we could add Menu or something on top of the picker but that could become quite cluttered. Maybe we could add a small pane between the prompt and the options for completion candidates.

@pascalkuthe
Copy link
Member

I think selecting from history it could just be a keybinding like a-n/a-p or a-up/a-down or similar. The old.prompt doesn't shown a history either.

We need to streamline the picker keybindings to find space for that they are cluttered right now and filled woth duplicates

@the-mikedavis
Copy link
Member

I marked #11209 as closing this since the main focus from the description is on adding to history. The discussion about selecting things other than the most recent history is also valuable though, I'll add another issue about that and link back here

@xJonathanLEI
Copy link
Contributor Author

Tested #11209 and it works. Not that this is significant but 2 subtle differences this has with the old search history experience are:

  1. With this new approach, each time you use <space> ' to bring up the last picker and grab an entry from there, the history is updated. Previously this only happened when you issue the search.

  2. This one is slightly more significant: you don't get the history entry when you issue a search, and no result shows up and you hit <enter>. This could be a bit troublesome in combination with Initial delay in the new global search experience #11206, where a user might miss by typing too fast and hitting <enter> too early before the debounce delay. A seemingly valid workaround is to use the "last picker" feature by pressing <space> ', but unfortunately, the last picker after being resurrected won't trigger a search: you'd end up with the empty list and you'd have to do something like adding a random character and deleting it to trigger the search.

@the-mikedavis
Copy link
Member

Not sure we can fix the first one cleanly but the second one we can just move the editor.registers.push call out of the if let Some(option) = self.selection() block

@xJonathanLEI
Copy link
Contributor Author

Great! I think 1 is pretty much just a harmless difference in behavior. So I'd say it's fine. It's just like breaking muscle memory, which takes a bit getting used to but is not inferior in anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants