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

Quick Search - consider removing the space after % for text search #191503

Closed
andreamah opened this issue Aug 28, 2023 · 13 comments · Fixed by #197028
Closed

Quick Search - consider removing the space after % for text search #191503

andreamah opened this issue Aug 28, 2023 · 13 comments · Fixed by #197028
Assignees
Labels
insiders-released Patch has been released in VS Code Insiders polish Cleanup and polish issue search Search widget and operation issues
Milestone

Comments

@andreamah
Copy link
Contributor

We currently require % and a space to use the search quick access. Although files could start with % (noted in #189964 (comment)), we should also consider how often this would be the case.

Other picks like the command palette don't have a space, so perhaps it's expected to not have a space.

@andreamah andreamah added polish Cleanup and polish issue search Search widget and operation issues labels Aug 28, 2023
@andreamah andreamah added this to the September 2023 milestone Aug 28, 2023
@andreamah andreamah self-assigned this Aug 28, 2023
@gjsjohnmurray
Copy link
Contributor

I contribute to an extension the implements a FileSearchProvider onto code environments (InterSystems IRIS and relatives) where "files" whose names begin with % are common since that naming convention is used for library packages. It would be a problem for our users if an attempt to quick-open, say, %Foo.Bar.cls failed because the request was now interpreted as a TextSearchProvider quick-search instead.

Pinging @isc-bsaviano for info.

@isc-bsaviano
Copy link

John described the concern perfectly. This will lead to a lot of confusion for our users when the FileSearchProvider and TextSearchProvider APIs are finalized (which we hope is soon!). I see how having the space is inconsistent, so is there another "symbol" character that can be used other than %? Or is there a clear way to "escape" that % so it's not a search trigger?

@gjsjohnmurray
Copy link
Contributor

is there another "symbol" character that can be used other than %?

How about = instead?

@andreamah
Copy link
Contributor Author

John described the concern perfectly. This will lead to a lot of confusion for our users when the FileSearchProvider and TextSearchProvider APIs are finalized (which we hope is soon!). I see how having the space is inconsistent, so is there another "symbol" character that can be used other than %? Or is there a clear way to "escape" that % so it's not a search trigger?

You can still search for % using "%" if we decide not to have a space. Would that be okay?

@isc-bsaviano
Copy link

You can still search for % using "%" if we decide not to have a space. Would that be okay?

To quick open files that start with %, users would have to enter "%Foo.Bar.cls" instead of %Foo.Bar.cls? Or would it be "%"Foo.Bar.cls?

@andreamah
Copy link
Contributor Author

andreamah commented Aug 31, 2023

You can still search for % using "%" if we decide not to have a space. Would that be okay?

To quick open files that start with %, users would have to enter "%Foo.Bar.cls" instead of %Foo.Bar.cls? Or would it be "%"Foo.Bar.cls?

"%Foo.Bar.cls

@andreamah
Copy link
Contributor Author

andreamah commented Aug 31, 2023

It's a little tricky to find a symbol that nobody would use in a file. Is % frequently used to prefix your files?
edit: for some reason, I missed some of the above comments. That makes sense. I wonder if there's another symbol that is usually invalid in filenames. '=' seems sort of awkward in this situation imo

@andreamah
Copy link
Contributor Author

andreamah commented Sep 1, 2023

I talked to the team about this - a popular idea was to just do search ("search" and a space) instead of a symbol. Sort of like debug . That way, it's more clear and follow an existing pattern we have.

@andreamah
Copy link
Contributor Author

andreamah commented Sep 7, 2023

I've been talking to team members about this whole "what should trigger quick search" issue since it's a little tricky to find something that intuitive and wouldn't conflict with user experiences. Some notes from this discussion:

  • Having %+<space> isn't consistent with any of the behavior that we have for symbols that trigger quick accesses (ie: @)
  • We could use a keyword like search + <space> like we have debug + <space> for the debug quick access, but we think that having a whole word would be hard for future use cases like having something before the trigger symbol/word. For example, you can have <word_to_query_file>@<word_to_query_symbols_in_file>
image

Similarly, we can have <word_to_query_file>%<word_to_query_text_in_file> if we used a percentage, and this would look really awkward if this % was replaced with the string search

  • Exploring other characters, ones like ~, $, etc. seemed quite common in filenames. We considered =, but that sort of seemed like it was calculating something; we also considered ^, but it would be confusing if we were to introduce regex to the quick search. Any opening/closing bracket or brace would look odd without the other.
  • To escape this behavior, you can actually use a space before the character to search for it (I was wrong above- although you can use a " to escape, you can also use <space> character. So %Foo.Bar.cls to escape, which is a lot easier than "Foo.Bar.cls). We should probably document this somewhere if isn't already documented.

From this, using % without a space seemed like the best way to go forward.

Additional things discussed:

  • Why is the quick chat not triggered like quick access commands? The other command center options do so.
    • This is because quick chat is a special case and the input used in chat is quite different from the basic one in the quick open. Its UX is also much different from the other quick accesses. When developing a feature for the command center, anything that can be made as a quick access should aim to follow the pattern of most quick accesses (<trigger symbol/word> triggering the feature).
    • In the case of quick search, it should also be triggered like other quick accesses because it allows you do <word_to_query_file>%<word_to_query_text_in_file> as mentioned above.
  • The command center currently says Search by default, but it only searches files - in the future, it should be pretty intuitive that it searches files by default, not text in files.

cc @TylerLeonhardt @Tyriar @daviddossett

@gjsjohnmurray
Copy link
Contributor

I guess it'll be tolerable for our users to have to prefix with a space if they're trying to quickopen something beginning with %.

@isc-bsaviano
Copy link

Since quick search is being actively refined, is there any chance of TextSearchProvider (#59921) making the jump to stable soon?

@andreamah
Copy link
Contributor Author

andreamah commented Sep 8, 2023

I have been meaning to finalize FileSearchProvider and TextSearchProvider as soon as I can, but asks from the team are taking some precedence. For example, quick search was originally introduced so that it can possibly used for AI integration in the future (which is a higher team priority).

@isc-bsaviano
Copy link

Thanks for the update, I'm glad to hear that they are still on your radar!

@andreamah andreamah modified the milestones: October 2023, November 2023 Oct 23, 2023
andreamah added a commit that referenced this issue Oct 30, 2023
Quick Search - consider removing the space after `%` for text search
Fixes #191503
@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Oct 30, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
insiders-released Patch has been released in VS Code Insiders polish Cleanup and polish issue search Search widget and operation issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants