Skip to content

Conversation

@ErichDonGubler
Copy link

@ErichDonGubler ErichDonGubler commented May 15, 2022

Intended to resolve #51, pending discussion (there's some trade-offs!). See individual commits for more details on changes.

Open questions:

  • What new tests should be added? Cases that I see:
    • The case mentioned in the first commit (refactor: skip duplicate canonicalized paths) is a good candidate.

Do our best to limit visiting paths to at most once, in preparation for
allowing multiple paths to be specified in a search-and-replace
operation. If we didn't do this, multiple path cases like this would be
handled incorrectly:

```
$ cat a.txt
foo

$ # We want to replace `foo` with `foobar`...
ruplacer foo foobar a.txt a.txt
<snip>

$ # ...but we got `foobarbar` instead:
$ cat a.txt
foobarbar
```

It's important to note what the trade-off for deduplication is. For each
path, we now call `std::fs::canonicalize`, and put the canonicalized
path in a set built using the the [`patricia_tree` crate] for somewhat
efficiently storage.

Interestingly, the above is also a potential issue with following
symlinks, which has never been allowed before. Perhaps this change could
make it sufficiently safe to add an option for following symlinks?

[`patricia_tree` crate]: https://docs.rs/patricia_tree/0.3.1/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Accept multiple paths on the command line

1 participant