Draft: Add FD_CONFIG_PATH environment variable #1218
Conversation
5088550 to
90fed03
Compare
901d5dc to
a9d4421
Compare
57da6d6 to
93da15b
Compare
Summary: Currently, `--ignore-vcs` only serves to unset `--no-ignore-vcs`. There is currently no way to tell fd to respect gitignore files when not in a git repository. This commit adds the flag `--no-require-git` to make fd always respect all gitignore files. This behaves the same as the `--no-require-git` option in [ripgrep](https://github.com/BurntSushi/ripgrep/blob/3bb71b0cb8727ac43237af78ba5c707298de0128/crates/core/app.rs#L2214-L2226) This commit also contains an unrelated wording fix to CONTRIBUTING.md Test Plan: `tests/tests.rs` Background: I am using [Sapling](https://sapling-scm.com/docs/introduction/) for working with git repositories (including this commit☺️ ). Since Sapling uses `.sl` instead of `.git`, tools using the `ignore` crate (rg and fd) would show gitignored files. I made a patch (vegerot/ripgrep@ebf17ee) to `ignore` to respect gitignores with _either_ `.git` or `.sl`. However, @BurntSushi said he did not want to merge that patch and instead suggested I use `--no-require-git` (BurntSushi/ripgrep#2374). This works fine, but I couldn't use this workaround for my other favorite tool! That's what this patch is for 😁 (a follow-up patch will add a similar `FD_CONFIG_PATH` environment variable like `RG_CONFIG_PATH`)
Summary: Add FD_CONFIG_PATH environment variable which points to a file with config options that are prepended to any `fd` command. This has the same behavior as `RG_CONFIG_PATH` from ripgrep Test Plan: `cargo test` TODO: - add flag `--no-config` to not respect FD_CONFIG_PATH, `.fdignore`, or global - add documentation fd ignore - make parsing config file blazingly fast - if FD_CONFIG_PATH not set, or file does not exist, just use `args_os()` directly - fix `test_invalid_cwd`
| args.insert(0, cli_args.next().unwrap()); // [/usr/bin/fd, --no-require-git] | ||
|
|
||
| // this way, CLI arguments override config-file arguments | ||
| args.extend(cli_args.skip(1)); // [/usr/bin/fd, --no-require-git, --hidden, --etc] |
There was a problem hiding this comment.
perhaps:
cli_args.splice(1..1, args.into_iter())?| // this way, CLI arguments override config-file arguments | ||
| args.extend(cli_args.skip(1)); // [/usr/bin/fd, --no-require-git, --hidden, --etc] | ||
|
|
||
| let mut matches = Opts::command().get_matches_from(args); |
There was a problem hiding this comment.
I think you could just do Opts::parse_from(args) for this and onward.
| }; | ||
|
|
||
| // TODO(vegerot): 🚀 | ||
| match std::fs::read_to_string(config_path) { |
There was a problem hiding this comment.
Do we want to support comments in this file? Or allow leaving of the beginning "--"? Or restrict which options are allowed in here?
| @@ -161,6 +161,14 @@ impl TestEnv { | |||
| } | |||
| } | |||
|
|
|||
| pub fn set_environment_variable(&self, key: &str, value: &str) { | |||
There was a problem hiding this comment.
These functions might not work well when multiple tests are run in parallel. Right now, it won't be an issue, because only a single test is using these functions. But if there are multiple tests, then one test might set this environment variable in one thread, then another test will overwrite that in another thread before the first test has a chance to execute fd. It would probably be better to set the enviornment when we spawn the fd process directly.
There was a problem hiding this comment.
Actually... I think it could be a problem currently, because the test that uses FD_CONFIG_FILE could run at the same time as another test, and result in that test starting with the config file set, and inheriting the options from the config file.
|
I'm closing this due to inactivity. Please feel free to comment in case it should be re-opened. |
Summary: Add FD_CONFIG_PATH environment variable which points to a file with config options that are prepended to any
fdcommand.This has the same behavior as
RG_CONFIG_PATHfrom ripgrepTest Plan:
cargo testTODO:
--no-configto not respect FD_CONFIG_PATH,.fdignore, or globalfd ignore
args_os()directly
test_invalid_cwd