helix-editor / helix

A post-modern modal text editor.
https://helix-editor.com
Mozilla Public License 2.0
32.8k stars 2.42k forks source link

Global Picker trims? search input #11404

Closed hnorkowski closed 1 month ago

hnorkowski commented 1 month ago

Summary

The local and global search show different results for the same search team. My hypothesis is that the global search trims the input.

Reproduction Steps

I tried this: 1. open `main.rs` with `hx` in a new rust crate 2. enter the following content: ```rs fn main() { let _ = 1 > 2; let _ = match 1 { 1 => 2, _ => unreachable!(), }; } ``` 3. do a local search for ` > 2`. It will only match in line 2 4. open global search and press enter to reuse the same search pattern #### I expected this to happen: It should show the exact same match (in line 2) as the local search #### Instead, this happened: It shows 2 matches: lines 2 and 4 If enter local search again after you did the global search you see that the search pattern got changed: the leading space got removed. ### Helix log
~/.cache/helix/helix.log ``` please provide a copy of `~/.cache/helix/helix.log` here if possible, you may need to redact some of the lines ```
### Platform Linux ### Terminal Emulator alacritty ### Installation Method arch AUR ### Helix Version helix 24.7 (cfe80acb)
RoloEdits commented 1 month ago

Should be the trim here: https://github.com/helix-editor/helix/blob/0a4432b104099534f7a25b8ea4148234db146ab6/helix-term/src/ui/picker/query.rs#L57-L69

edit:

Removed both and got only one match now: image

the-mikedavis commented 1 month ago

The trim is intentional. If you want to match on space you should use the regex [ ] or \s

RoloEdits commented 1 month ago

Having different semantics for searching between local and global seems like unneeded user friction. Wonder if this can be cleaned up a bit? The main culprit here is spaces. Can these not be special cased?

hnorkowski commented 1 month ago

The trim is intentional. If you want to match on space you should use the regex [ ] or \s

That's a terrible experience. Why would you ever intentionally trim a regex input? Esp. if the content is already in the search register. I think people using a terminal editor could be considered smart enough to know that a leading space makes a difference and don't need to babysitted like that.

Having different semantics for searching between local and global seems like unneeded user friction.

100% agree.

the-mikedavis commented 1 month ago

The input for local and global search is handled differently, for example you can't use column filters like %path foo.rs in local search. The trim is there so you can write column filters separated from the other input. For example foo %flags + in the buffer picker to search for a buffer name matching foo with the + flag. Making the trim smarter - only trimming the spaces around column names - is ok but we should not remove the trimming entirely. And we shouldn't special case global search so that the input is parsed differently than any other picker - that special-casing is a terrible experience.

RoloEdits commented 1 month ago

That's a terrible experience. Why would you ever intentionally trim a regex input?

Its due to the fact that you can have filtering as well, for example > 2 %path foo.rs. The first space is significant to the search, but the one before %path is a separator. The picker is also used in more places than just the global search, like the symbol search, for example. Where multiple column filters can exist between search patterns.

image

RoloEdits commented 1 month ago

As I wrote in the draft:

A proposition I would offer would be to exclude the prior space before and after a % filter. This would involve a local variable to keep track of this state, was_prior_pattern_a_filter: bool and if so, exclude the first space after the filter. This would allow leading spaces in the search pattern.

Additionally, adding a forwards check to see if the next non-space char is a %, and then excluding the final space before the %. This would allow searches to include trailing spaces.

Is this a viable solution? A one-space-only trim specialization if before or after a filter pattern.

hnorkowski commented 1 month ago

The trim is there so you can write column filters separated from the other input.

Ok yeah your answer sounded more like "we trim the search term intentionally".

IMO this should be changed. Currently, its not a great experience. I think the ideas of @RoloEdits sound good!