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

Can't disable adapters #183

Closed
runxel opened this issue Sep 14, 2023 · 12 comments · May be fixed by #256
Closed

Can't disable adapters #183

runxel opened this issue Sep 14, 2023 · 12 comments · May be fixed by #256
Labels
bug Something isn't working

Comments

@runxel
Copy link

runxel commented Sep 14, 2023

Describe the bug
Trying to disable adapters with --rga-adapters=-zip,sqlite does not work.
Next search will still have .zip's being searched...
(not helpful at all, it will just spit a lot of errors like:)

adapter: zip
Error: Unsupported Zip archive: The file length is not available in the local header

Operating System and Version
MacOS 13.5.2

Output of rga --version
ripgrep-all 0.9.6

@runxel runxel added the bug Something isn't working label Sep 14, 2023
@phiresky
Copy link
Owner

It seems to work for me on 1.0.0-alpha. Please try there

@runxel
Copy link
Author

runxel commented Sep 14, 2023

Nope, doesn't work.
Gives me now preprocessor command failed errors for zips tho.

Also the whole output is now cluttered with

<somepath>/.DS_Store adapter: postprocprefix

Huh??? Other file types as well. With Alpha 5 it seems rga just looks into everything.

@phiresky
Copy link
Owner

I can't reproduce this. If I run rga --rga-adapters=-zip,sqlite fell in the exampledir in the repo, the result is as expected. Please send a minimal reproducible example.

@runxel
Copy link
Author

runxel commented Sep 16, 2023

Please send a minimal reproducible example.

How can I do that?
Also, is this config saved somewhere so I can look into that?

@lafrenierejm
Copy link
Contributor

Please send a minimal reproducible example.

How can I do that?

@runxel In this case, a minimum working example would mostly consist of you sharing a file that results in the buggy behavior when running rga against it. If your original file is large or contains sensitive information, you might need to construct a new file for testing. If you're willing to do a little extra work, you could even raise a PR adding that new file to exampledir/ with corresponding tests that fail.

Also, is this config saved somewhere so I can look into that?

Yes. rga uses directories-next, so the location of the config file varies by OS:

  • macOS: ~/Library/Application Support/ripgrep-all/config.jsonc
  • Linux: $XDG_CONFIG_HOME/ripgrep-all/config.jsonc or ~/.confic/ripgrep-all/config.json if $XDG_CONFIG_HOME isn't set
  • Windows: %AppData%/ripgrep-all/config.json

@runxel
Copy link
Author

runxel commented Nov 30, 2023

Thank you for that @lafrenierejm. So it seems the issue is with rga not updating its config.json at all. Mine is empty except for the default comment in it.
How would the entry need to look like? I could then see if rga at least can read its own config file.
Because this does not work:

{
	"adapters": "-zip"
}

@phiresky
Copy link
Owner

phiresky commented Jan 15, 2024

Both --rga-adapters=-zip and "adapters": ["-zip"] in the config file work (for me) in the newly released 0.10.3.

@runxel
Copy link
Author

runxel commented Aug 4, 2024

Coming back to this because this does not work at all in 0.10.6. @phiresky
For other stumbling upon this: Putting in "adapters": "-zip,sqlite", does not work; RGA will nag that it is not an array. Okay, easy fix, "adapters": ["-zip,sqlite"] does the trick. Or so I thought.
But now I get this error when running rga: Error: Could not remove adapter zip,sqlite: Not in list.
Looks like I can't win. :D

@phiresky
Copy link
Owner

phiresky commented Aug 4, 2024

the syntax is probably ["-zip", "sqlite"] though i didn't check

@runxel
Copy link
Author

runxel commented Aug 5, 2024

the syntax is probably ["-zip", "sqlite"] though i didn't check

Yes, that works! Thanks @phiresky!
Even tho I have to say that this is a pretty bad design, not only because this not documented anywhere, but it is so unintutive that the first item has the - (minus) in the front, but the following, which should also be excluded, does not. 🤔

@phiresky
Copy link
Owner

phiresky commented Aug 9, 2024

creating a list that can be both additive as well as overriding is challenging, and then getting intuitive support for the same syntax in a config file as well as cli args while keeping it comfortable is even more challenging ;) esp when the config format is added later than the cli syntax

@lafrenierejm
Copy link
Contributor

creating a list that can be both additive as well as overriding is challenging, and then getting intuitive support for the same syntax in a config file as well as cli args while keeping it comfortable is even more challenging ;) esp when the config format is added later than the cli syntax

Since ripgrep-all isn't yet at 1.0.0, what would you think about making a breaking change to the config file schema to have two distinct lists? I'm thinking they could be something like:

  • adapters_enable: List of adapters to enable in addition to any default adapters.
  • adapters_disable: List of adapters to disable. This can be used to disable default adapters. Entries in this list take priority over entries in adapters_enable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants