Repository: sharkdp/fd
Pull request #509 (2019-11-20 10:02:11)

#475 - Allow changing current working directory of fd
Hi, this PR solves #475 by implementing `--base-directory` option and using `std::env::set_current_dir`. Feedback appreciated.

sharkdp: Looks good, thank you.

Can we please implement a simple test for this option?
hajdamak: Sure. Pull request updated.
Is this OK?
hajdamak: Is this fine to merge it?
hajdamak: My current understanding is as follows:
`--base-directory` only applies if positional `path` argument is relative or is not passed (which means relative `.`). Changing current working directory only influences relative paths used, so if `path` is absolute `--base-directory` does not change anything.
When `--base-directory` is used with relative `path` it just means that `path` is resovled relative to the directory passed as `--base-directory`.
`--search-path` works the same as positional `path` argument so everything above applies. 
`--full-path` is not influenced by `--base-directory` in any way, it just causes pattern to be applied on full path (instead of file name) for all the files that reside in `path` or `--search-path`.
So in summary `--base-directory` influences how relative paths passed ass `path`s or `--search-path`s are resolved.
When mutliple `path`s or `--serach-path`s are passed, `--base-directory` only applies to relative ones.
Does this make sense?

sharkdp: >  `--base-directory` only applies if positional `path` argument is relative or is not passed (which means relative `.`). Changing current working directory only influences relative paths used, so if `path` is absolute `--base-directory` does not change anything.

:+1:



>  When `--base-directory` is used with relative `path` it just means that `path` is resovled relative to the directory passed as `--base-directory`.

right.

>  `--search-path` works the same as positional `path` argument so everything above applies.

makes sense.

>  `--full-path` is not influenced by `--base-directory` in any way, it just causes pattern to be applied on full path (instead of file name) for all the files that reside in `path` or `--search-path`.

Yes, right. Because `--full-path` always searches on the absolute path.

>  So in summary `--base-directory` influences how relative paths passed ass `path`s or `--search-path`s are resolved.

There is one more thing that is potentially very confusing. Consider the following (let's say we are inside the `fd` repository directory):
```
❯ fd input
src/exec/input.rs

❯ fd input src/
src/exec/input.rs

❯ fd input --base-directory=src/
exec/input.rs

❯ fd input src/ --base-directory=src/
[fd error]: 'src/' is not a directory.
```

I guess this happens because `fd` is now trying to resolve the search path (`src/`) relative to the base directory (also `src/`), resulting in a lookup for `src/src`.

I guess we should wait with changing the directory until we have resolved other (relative) path arguments.

We should also add a test for this.
hajdamak: > I guess we should wait with changing the directory until we have resolved other (relative) path arguments.

The question is whether we want to keep `path` relative to current working directory where `fd` was executed or the passed `--base-directory`.  I would rather stay with latter because it seems to have more use cases. In that case only error message should be changed to make it less confusing, for example also show how provided `path` is resolved relative to `--base-directory`.
What do you think?
sharkdp: > The question is whether we want to keep `path` relative to current working directory where `fd` was executed or the passed `--base-directory`.

I think it should be relative to the CWD of the terminal (i.e. where `fd` was executed). This is how paths are passed to programs. This is how the shell completion in terminals works. Everything else is confusing, IMO.
hajdamak: Yeah, I get your point. Especially that docs in `path` state: 

> The directory where the filesystem search is rooted (optional). If omitted, search the current working directory.

This in practice means that `--base-directory` only works if no `path` or `--search-path` argument is provided.
sharkdp: > > The directory where the filesystem search is rooted (optional). If omitted, search the current working directory.
> 
> Such an approach in practice means that `--base-directory` only works if no `path` or `--search-path` argument is provided.

Oh.. you mean because we would search the `<base-directory>` instead of the current directory if no path is supplied?

I think the following is a reasonable use case:
```
fd --base-directory=<base-path> pattern <base-path>/<search-path-below-base>
```
The `<base-path>/<search-path-below-base>` path would be searched, but all results are relative to `<base-path>`.

I think we should also look into what other programs do. Like `git` with its `-C <path>` option.
hajdamak: > Oh.. you mean because we would search the base-directory instead of the current directory if no path is supplied?

Yeap. And if path is supplied `--base-directory` will have no effect because paths are resolved relative of CWD of `fd` instead of `--base-directory`.

> I think the following is a reasonable use case:
fd --base-directory=base-path pattern base-path/search-path-below-base
The base-path/search-path-below-base path would be searched, but all results are relative to base-path.

This is the use case I had in mind when opted for making paths resolved relative to `--base-directory` but with following usage: 
`fd --base-directory=<base-path> pattern <search-path-below-base>`

> I think we should also look into what other programs do. Like git with its -C <path> option.

In Git using `-C` causes other relative paths passed as command parameters to be resolved relative to `-C` directory. This is what man states in `-C` section:

> This option affects options that expect path name like --git-dir and --work-tree in that their interpretations of the path names would be made relative to the working directory caused by the -C option. For example the following invocations are equivalent:
git --git-dir=a.git --work-tree=b -C c status
git --git-dir=c/a.git --work-tree=c/b status

I still think having `--base-directory` just change CWD of the process with all the consequences that it has on passed relative paths is more simple and straightforward approach though confusing at first. In docs we could just write that it changes CWD so `fd` runs as if executed in different directory and passed paths are resolved relative to it.
sharkdp: > In Git using `-C` causes other relative paths passed as command parameters to be resolved relative to `-C` directory. This is what man states in `-C` section:

Ok, then let's do it the same way and keep the logic as is.

> I still think having `--base-directory` just change CWD of the process with all the consequences that it has on passed relative paths is more simple and straightforward approach though confusing at first.

:+1: 



> In docs we could just write that it changes CWD so `fd` runs as if executed in different directory and passed paths are resolved relative to it.

Sounds great. Looking forward to merging this, if you could extend the documentation in that respect.
hajdamak: Great. Docs updated. Are they from you POV?

Patch:
diff --git a/src/app.rs b/src/app.rs
index f42ac8c..645c7d6 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -257,6 +257,14 @@ pub fn build_app() -> App<'static, 'static> {
                 .hidden_short_help(true)
                 .overrides_with("show-errors"),
         )
+        .arg(
+            arg("base-directory")
+                .long("base-directory")
+                .takes_value(true)
+                .value_name("path")
+                .number_of_values(1)
+                .hidden_short_help(true),
+        )
         .arg(arg("pattern"))
         .arg(
             arg("path-separator")
@@ -443,5 +451,10 @@ fn usage() -> HashMap<&'static str, Help> {
         , "(hidden)"
         , "Provide paths to search as an alternative to the positional <path> argument. \
            Changes the usage to `fd [FLAGS/OPTIONS] --search-path <path> --search-path <path2> [<pattern>]`");
+    doc!(h, "base-directory"
+        , "(hidden)"
+        , "Change current working directory of the fd process to provided path. \
+           Non-absolute paths passsed to positional <path> argument or '--search-path' will be \
+           resolved relative to this directory instead of directory where fd was executed.");
     h
 }
diff --git a/src/main.rs b/src/main.rs
index 426eb08..e17dc7d 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -44,6 +44,23 @@ fn main() {
     let checked_args = transform_args_with_exec(env::args_os());
     let matches = app::build_app().get_matches_from(checked_args);
 
+    // Set the current working directory of the process
+    if let Some(base_directory) = matches.value_of("base-directory") {
+        let basedir = Path::new(base_directory);
+        if !fshelper::is_dir(basedir) {
+            print_error_and_exit!(
+                "The '--base-directory' path ('{}') is not a directory.",
+                basedir.to_string_lossy()
+            );
+        }
+        if let Err(e) = env::set_current_dir(basedir) {
+            print_error_and_exit!(
+                "Could not set '{}' as the current working directory: {}", 
+                basedir.to_string_lossy(), e
+            );
+        }
+    }
+
     // Get the search pattern
     let pattern = matches.value_of("pattern").unwrap_or("");
 


Test patch:
diff --git a/tests/tests.rs b/tests/tests.rs
index d9c0130..7237252 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -1350,3 +1350,53 @@ fn test_custom_path_separator() {
         one=two=three=directory_foo",
     );
 }
+
+#[test]
+fn test_base_directory() {
+    let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);
+
+    te.assert_output(
+        &["--base-directory", "one"],
+        "b.foo
+        two
+        two/c.foo
+        two/C.Foo2
+        two/three
+        two/three/d.foo
+        two/three/directory_foo",
+    );
+    
+    te.assert_output(
+        &["--base-directory", "one/two", "foo"],
+        "c.foo
+        C.Foo2
+        three/d.foo
+        three/directory_foo",
+    );
+
+    // Explicit root path
+    te.assert_output(
+        &["--base-directory", "one", "foo", "two"],
+        "two/c.foo
+        two/C.Foo2
+        two/three/d.foo
+        two/three/directory_foo",
+    );
+
+    // Ignore base directory when absolute path is used
+    let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES);
+    let abs_base_dir = &format!("{abs_path}/one/two", abs_path = &abs_path);
+    te.assert_output(
+        &["--base-directory", &abs_base_dir, "foo", &abs_path],
+        &format!(
+            "{abs_path}/a.foo
+            {abs_path}/one/b.foo
+            {abs_path}/one/two/c.foo
+            {abs_path}/one/two/C.Foo2
+            {abs_path}/one/two/three/d.foo
+            {abs_path}/one/two/three/directory_foo",
+            abs_path = &abs_path
+        ),
+    );
+
+}