Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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.

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

Successfully merging a pull request may close this issue.

3 participants