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

remove the --no-config flag for ripgrep #194101

Closed
bartroozendaal opened this issue Sep 26, 2023 · 15 comments
Closed

remove the --no-config flag for ripgrep #194101

bartroozendaal opened this issue Sep 26, 2023 · 15 comments
Assignees
Labels
info-needed Issue requires more information from poster

Comments

@bartroozendaal
Copy link

bartroozendaal commented Sep 26, 2023

TL;DR As a developer I want to use the ripgrep config file to tailor searching large repositories so I can get the results that I need

vscode uses ripgrep to perform search operations. When launching ripgrep, vscode passes a flag --no-config to not interpret any settings a user may have in a ripgrep-config file. Any folders to exclude from the search are also passed as flag to ripgrep.

These are the places where the no-config flag is set:
https://github.com/search?q=repo%3Amicrosoft%2Fvscode%20--no-config&type=code

Where there is a large number of folders to exclude from the search (e.g. through .gitnore settings or search.exclude settings in the vscode config), launching ripgrep may give a ENAMETOOLONG error (since the total command is too long for the shell to handle).

See issue #869 One reason to have a long list of folders to exclude, may come from vscode inability to efficiently exclude certain folders from search (and the explorer for that matter). Could we use the ripgrep config file, we could more efficiently exclude folders from the search.

Also related is #134415

Note 1: it would be even better if vscode itself would use the config file to exclude folders (and maintain it), but I can see that that feature request would also need the same syntax support to exclude folders from the file explorer and that may be a bit too much to ask for.
Note 2: I'd be happy to create a PR for removing the --no-config flag.
Note 3: Just did a quick test with the --no-config flag removed from the code, and it seems to work just fine without any side effects.

@andreamah
Copy link
Contributor

See issue #869 One reason to have a long list of folders to exclude, may come from vscode inability to efficiently exclude certain folders from search (and the explorer for that matter). Could we use the ripgrep config file, we could more efficiently exclude folders from the search.

If we implemented negative globbing as mentioned in #134415, then would this generally solve the issue of being unable to create proper queries? If not, what is an example that would be simpler if passed directly into ripgrep?

Where there is a large number of folders to exclude from the search (e.g. through .gitnore settings or search.exclude settings in the vscode config), launching ripgrep may give a ENAMETOOLONG error (since the total command is too long for the shell to handle).

We probably plan to adopt the config file to prevent ENAMETOOLONG (#98514), although having it user-configurable is still arguable because lots of args could easily break how we parse the rg output.

@andreamah andreamah added the info-needed Issue requires more information from poster label Sep 27, 2023
@bartroozendaal
Copy link
Author

About the configuration file being configured by the user: I would expect that any flags passed 'on the command line' would take preference over the settings in the config file. So, even if a user would set something that could influence the parser, the command line flags would override those and you would prevent that from happening. I haven't tested that, but that would be my expectation. So, I don't think there would be a risk there. All flags can remain as is to make this happen: only --no-config would need to be removed.

As for implementing negative globbing: yes, that would make it much easier. E.g.

give a folder like:

/src/path1
/src/path2
/src/path3

If you would want to only include path /src/path2, you now have to pass the following to search.exclude/files.exclude:

/src: false,
/src/path1: true,
/src/path2: false,
/src/path3: true

while if you would have negatives, you could do something like this:

!(/src/path2): true

In #869 you can find other proposals for syntax too.

@andreamah
Copy link
Contributor

If we implemented negative globbing in the editor (as proposed in #134415), would that be sufficient?

@bartroozendaal
Copy link
Author

Well, I'm an architect, so it depends ;-)

If !(/src/path2): true also means that any other child of /src/ would be included, and path2 would not, then the answer is yes. But to be really honest, I'm not quite clear what the proposal of 134415 is on what to do and how to interpret various combinations.

Basically related to this comment:
#134415 (comment)

What I would expect:

  1. The more precise specification takes precedence over lesser precies specification
  2. Exclude specification takes precedence over include specification

@andreamah
Copy link
Contributor

I think that, given the current progress on that thread, we do have excludes taking precedence over include, but I don't quite know if we say that more precision takes precedence over lesser precision. To be quite honest, I still think that we should go about this by trying to iron out our glob implementation to be more efficient rather than going straight to the ripgrep config file, since the root problem seems to be with the way that we handle globs.

In this case, I would say to contribute to the discussion in #134415 about this such that we can have an overall more uniform and clear way to handle globs in the future.

@bartroozendaal
Copy link
Author

I'd be happy to (try to) contribute to that thread, but is that still actively being considered? It started in 2015 (!) and the last proper comment was in April 2022. I'd be happy if the glob.ts solution would be implemented and would solve the issue, but I'm not too sure if it is being picked up at all. The proposed solution for removing --no-config is a small change, most probably risk free, and would solve issues for people that work in large repos and want to exclude specific folders from searching.

Personally, I'd be ok with just removing the flag, and no maintenance of the config file at via vscode. That we can do manually. If my assumption is right that command line flags take preference over config file settings, then I'd say there would be zero risk for vscode.

@andreamah
Copy link
Contributor

To be honest, this is my view on it:

  • We try not to introduce things to the product that will only have short-term use unless it's really necessary. In this case, once we iron out this globbing issue, allowing users to edit the config file directly would likely not be as needed. Then, taking it out would be difficult if people still use it.
  • I can't guarantee that we'll take immediate action on the globbing issue, since the team might not have the bandwidth to immediately handle it.

Knowing this, I totally understand that this is frustrating. But I would rather see a more concrete fix for this problem rather than something more temporary.

@bartroozendaal
Copy link
Author

Luckily, spending bandwidth is not my call to make ;-). But, since this is an open issue since 2015 (!) maybe it is time to pick it up.

When it really frustrates me (it might, because I'm running into this issue on a daily basis), I could clone the repo, make the changes, and make sure my local build gets merged with the latest master every night or so. Quick workaround is to only search in a given folder, disabling excluding the gitignore and search.exclude settings in the search dialog. But that is not ideal.

@bartroozendaal
Copy link
Author

I'd still would like to cycle back and give it another go, and make the argument again. People that have a lot of include/excludes run into issues because the number of excluded paths result in the ENAMETOOLONG error. Given the issue I referenced (#869) is there since 2015, and you also indicated that the 'better' solution probably won't be picked up any time soon, I don't think the argument against my proposal ('it is a short term solution') holds.

There may be not a lot of people suffering from this issue, but there must be some. I also think it is very possible to implement a setting, with a default that keeps the current behavior, but also gives users that run into this issue a way around it. I see the Github copilot extensions suffer from it, the npm extension suffers from it and search itself is broken (even if you know the way around that, it is extremely cumbersome).

What if I would create the PR and you guys then decide if the solution I have in mind is good enough and acceptable?

@andreamah
Copy link
Contributor

I reviewed this thread and now I'm thinking- have you tried using the .ignore configuration file to handle your ignores? The global ignore is located at $HOME/.config/git/ignore. The path is noted in the rg docs here. I believe that .ignore globs should take precedence over .gitignore.

@bartroozendaal
Copy link
Author

Thanks for looking into this again. You suggestion will probably work in the sense that ripgrep will ignore the files when searching. However, any entries there will also mean that git will ignore those files, and that is not what I want. I just want to exclude (a large set of) folders in a large repo from search and the explorer pane). They still need to be handled by git in a regular way.

Some background to make it make more sense maybe. We work in a large monorepo with +-100 teams sharing code. Each team obviously is mainly working on their own code. There is a code-owners file that indicates which team is responsible for what folders (which is used to automatically generate PR approval settings et cetera). I've written an extension that filters the monorepo so each team will only have a view on their files from the monorepo. I'm updating the files.exclude and search.exclude settings of vscode to do so.

However, because we don't have these negative globbing, the list of entries for both settings can become extremely long; for some teams too log for some extensions and for the command line to use ripgrep.

To get searching working I could add the settings I need to the ripgrep configuration, but since vscode is excluding a configuration file in launching ripgrep, that will not work.

So, my TL;DR summary/solution is:
Make the no-config flag for launching ripgrep optional, so any exclude settings in the ripgrep config file will be respected.

(My extension: https://marketplace.visualstudio.com/items?itemName=RoozendaalOnline.mymonorepo)

@andreamah
Copy link
Contributor

Thanks for looking into this again. You suggestion will probably work in the sense that ripgrep will ignore the files when searching. However, any entries there will also mean that git will ignore those files, and that is not what I want. I just want to exclude (a large set of) folders in a large repo from search and the explorer pane). They still need to be handled by git in a regular way.

Do .ignore files get ignored by Git? I thought that only .gitignore files were ignored by git, but I may be wrong.

@bartroozendaal
Copy link
Author

Yes, they do. I tried it ;-) I didn't know about it myself. It is a mechanism to add some files to be ignored locally as the .git folder doesn't get pushed to the repo

@VSCodeTriageBot
Copy link
Collaborator

Hey @andreamah, this issue might need further attention.

@bartroozendaal, you can help us out by closing this issue if the problem no longer exists, or adding more information.

@VSCodeTriageBot
Copy link
Collaborator

This issue has been closed automatically because it needs more information and has not had recent activity. See also our issue reporting guidelines.

Happy Coding!

@VSCodeTriageBot VSCodeTriageBot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 11, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

3 participants