Revisit search include/exclude and './' patterns

[roblourens]

Need to spend time thinking about the include/exclude pattern experience, the interactions between the include/exclude fields, .gitignore, and search/files.exclude, and whether ./ is needed or if there's a better way to achieve the effect, and how this all works in a multi-root world.

Related issues:

• Multi root: Cannot search specific file types anymore #29924
• Search no longer allows you to search sub-folders easily #29660
• Find in files files to include should override exclude settings? #23894
• Find in Files "files to include" override .gitignore #26574
• Search: unable to include/exclude via glob patterns #26634
• Find In Files in specific folder without opening the folder in VSCode #17164

[roblourens]

Problem: the search viewlet is complicated. It's not clear how to use the include/exclude boxes. Who really knows how the "Use Glob Patterns" buttons work? Multiroot makes it more complicated. Supporting absolute paths makes it more complicated. We can do better.

At the end of this brain dump is my actual proposal.

How does it currently work?

The include and exclude boxes each have a "Use Glob Patterns" button that puts them into a different mode. When the button is toggled off, foo is interpreted as {**/foo,foo/**}. This is useful because if foo is a folder name at the root of the current folder, then you search everything under foo/. If foo is a file, then you search every file named foo in the current folder. So it works as you might expect if you type *.ts or someFolder. When "Use Glob Patterns" is toggled on, it uses the patterns as-is, so you would have to type **/*.ts or someFolder/** to get the same effect. This mode lets you be more precise. But the name of the button is not quite right, because clearly you can use glob patterns in either case. It's just that the second mode requires you to write out the full glob pattern to match the files you want.

Currently, in single-root mode, "Find in Folder" puts ./foo/bar in the include box and this is used as a glob pattern. In multi-root mode, it puts the absolute path in the include box. Absolute paths are treated specially apart from other include patterns, and the search starts in that folder.

What should "Find in Folder" put in the include box?

Strategy	Pros	Cons
Absolute path	Totally unambiguous, and matches Sublime	The search viewlet is narrow, and an absolute path takes up an unnecessary amount of space.
Path fragment (root1/folder1)	Simpler and matches Atom. Ambiguousness is probably not a big problem most of the time	Ambiguous - what if there are two root folders with the same name? Also, foo/bar would currently match file bar under subfolder foo- do we change that? Someone's workflow will surely be disturbed
./ to match under workspace or single root folder, ./root1/folder1	Leaves foo/bar syntax alone. Matches what we already have in single root mode.	Different between single and multi-root modes. Typically . is the current folder, so is it clear that ./root1/folder1 is matching a root folder root1? Not sure anyone would discover this unless they already know about it. Has the same problem of ambiguity when multiple roots have the same name.
New syntax for multiroot, $/root1/folder1	Avoid ambiguousness of ./. I think this is inspired by TFS.	Maybe looks even stranger. Is it solving a real problem?
New UI, like a special label that represents the root folder	Would look great if we end up with some special UI elsewhere in vscode for root folder names	How do you type it, outside of using the "Find in Folder" UI? Introduce some kind of autocomplete or dropdown? Creates more problems

What should we do?

Find in Folder

For "Find in Folder", I'm leaning towards the ./ option. We can try it out and iterate. To avoid the ambiguity issue, "Find in Folder" can actually use an absolute path when there are two roots with the same name. For a single root workspace, it will work as it currently does.

Multi-root workspace:

root1/
  foo/
    …
root2/
  bar/
    …

I invoke "Find in Folder" on foo/. The include box is populated with ./root1/foo. If I search with ./foo in the include box, I see an error, "No folder in workspace named 'foo'". (But if I remove root2/ from the workspace, making this a single-root workspace, I would have to search with ./foo).

root1/
  foo/
    …
root1/
  bar/
    …
root2/
  baz/
    …

Now I have two root folders named root1. If I invoke "Find in Folder" on bar/, I get /Users/roblou/root1/foo. And if I do it on baz, I get ./root2/baz.

Glob patterns and paths

Currently we don't support mixing absolute paths with glob patterns, ie /foo/bar/**/*.ts. Ideally we would search / with an include pattern of /foo/bar/**/*.ts. But given BurntSushi/ripgrep#546, ripgrep doesn't optimize this by only searching in /foo/bar. So to be fast, we need to separate the directory that we are searching in (/foo/bar) from the include pattern that is also applied to the search (**/*.ts). Sublime deals with this by separating search paths and glob patterns, requiring you to use /foo/bar, **/*.ts which also works in VS Code. But since people expect to be able to mix them, we can try to be clever.

We can do it with this algorithm:

• Split the absolute path into path segments
• From the left, join all path segments until one is encountered that contains a glob char - [], {}, *, ?
  • For paths that contain glob chars, we need to decide how to escape them. \ won't work. They can be escaped in a glob pattern by wrapping in [], so maybe we reuse that syntax. Although that makes it look like it's part of a glob pattern when the point is to indicate that it isn't part of a glob pattern
• These joined segments on the left are the root folder for the search - everything else to the right is an include pattern that's also applied

Include/exclude boxes

Another problem is that the "Use Glob Patterns" buttons are confusing. Given the ./ syntax, we could actually get rid of it, using the following strategy.

• foo/bar => interpreted as {foo/bar/**, **/foo/bar}, like current behavior with "Use Glob Patterns" disabled, and applied under each root folder. So you might use this to search src for all src/ folders, or *.xml to search all XML files in the workspace.

• ./root1/foo/bar => as described above, searches within a particular root, starting the search in the specified subfolder in that root.

• ./root1/foo/ba{r,z}/**/*.js => follows the same rules as absolute paths for separating the search path from the glob applied inside that path.

The case that can't quite be covered, is when you want to search a multi-root workspace for something like a "Use Glob Patterns"-enabled include pattern, but you want to apply the same pattern to each root. For example, I have root folders a/ and b/, and I want to search src/**/*.ts in each one, without matching some other foo/src/…/bar.ts. Under this proposal, if I use src/**/*.ts, then that's interpreted as {**/src/**/*.ts, src/**/*.ts/**} and includes the foo/src that I don't want. Otherwise I can list out all the root folders, like ./a/src/**/*.ts, ./b/src/**/*.ts. Or, I could exclude foo/** if there is a particular pattern that I didn't want included. We could add some syntax like ./*/src/**/*.ts. I don’t really think it's a problem, so I think in general this would cover all the important scenarios for having "Use Glob Patterns" enabled or disabled.

Include/exclude precedence

Currently, exclude patterns always take precedence over include patterns, but ripgrep has more flexibility that we aren't taking advantage of - it determines precedence in patterns based on the order in which they are passed. Also, patterns passed in always take precedence over patterns from the gitignore.

In some cases we want includes to take precedence over excludes. If I have search.exclude with **/node_modules/, and I add an include pattern of node_modules, then I probably want to search in the node_modules folder. In other cases, I might have an exclude pattern of **/*.js, and an include of foo/**, and want the exclude to take precedence.

I think that typically users might use the 'include' box to limit the search or to override the .exclude settings, and they might use the 'exclude' box to override the 'include' box. So, we can apply the patterns in that order, 'exclude box' > 'include box' > '.exclude settings'.

Files excluded through settings

This box is not useful for determining which patterns are excluded through settings, not even in single-root mode. I can only see a small portion of the patterns that are applied. I don't think it's worth trying to create a good multi-root experience for it.

I think it's useful for one thing - reminding the user that there are some exclude settings applied. So I worry about removing it and having more people confused that results aren't found in some folders that are visible in the file explorer. But we do have the "Use Exclude Settings" button, and hopefully that would lead them down the right path. So, I think we should just remove it entirely.

[bpasero]

@roblourens some feedback from me:

• Find in Folder: +1 for trying to go with ./ even for multi root case
• Glob patterns and paths: +1 for keeping the input simple and allow for absolute paths that contain glob patterns with the algorithm you describe
• Include/exclude boxes: can we look at telemetry to see how many people actually have the button clicked? I do agree that the use case for these are very limited (+ @alexandrudima who imho suggested to remove them too)
• Include/exclude precedence: I like the idea of letting includes override excludes, but if we go down this path I think we need to apply a similar logic to the matching we do to out of workspace files and also for people that disable ripgrep. In other words, I think this is hard to accomplish given our glob matcher is not fit for treating includes over excludes and you might end up with differing results depending on options and files opened. For this case I actually like that I can just click the button to disable configured excludes in the search view UI
• Files excluded through settings: I find the hint still useful that settings also allow for configuring excludes. If the info is too prominent we could also think about showing it within the result message maybe?

[roblourens]

can we look at telemetry to see how many people actually have the button clicked?

No telemetry for the glob button, unfortunately

include/exclude precedence
In other words, I think this is hard to accomplish given our glob matcher is not fit for treating includes over excludes and you might end up with differing results depending on options and files opened.

I agree that it should be the same with or without ripgrep, and I think this is achievable using our glob matcher. Currently we match against the include and exclude patterns separately, and we just need to reorganize it to keep the exclude settings patterns separate from the viewlet exclude box, and test them in the right order.

Files excluded through settings
If the info is too prominent we could also think about showing it within the result message maybe?

I would be open to something as part of the results message, "btw there are files excluded through settings" with the button that links to settings.