D6271: rust-filepatterns: add a Rust implementation of pattern-related utils
kevincox (Kevin Cox)
phabricator at mercurial-scm.org
Wed Apr 24 05:59:00 EDT 2019
kevincox accepted this revision.
kevincox added a comment.
Thanks for the changes.
> Alphare wrote in filepatterns.rs:18
> I think I hit the same problem as the person in this issue: `https://github.com/rust-lang/regex/issues/451`. It's a hassle to escape bytes using the `regex` crate, and this felt like a good enough replacement.
Ah, I understand. This make sense then.
> Alphare wrote in filepatterns.rs:51
> It is required since `repl` is `&&[u8]`, which is not an iterator.
It is probably more clear to do `*repl` then to show that you are dereferencing it.
> Alphare wrote in filepatterns.rs:61
> This has a different behavior, since `position` expects a boolean return value in the closure for each element of the iterator.
I don't understand. The example I showed would give a boolean. The only difference I see is that the returned value would be 1 less (because .position() wouldn't count the skipped element) but this should be easy to handle in the code and I think would be more clear overall.
> Alphare wrote in filepatterns.rs:226
> While your version is just plain better, removing the regex-based comment escape breaks support for comments in `test-hgignore.t`, so this will still be necessary.
Oops, I should have pointed out that I just commented that out because I don't have access to the `regex` crate on the website. It should still be included in the code.
This looks good now.
To: Alphare, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel
More information about the Mercurial-devel