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.

INLINE COMMENTS

> 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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6271

To: Alphare, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel


More information about the Mercurial-devel mailing list