| Submitter | phabricator |
|---|---|
| Date | Aug. 26, 2019, 4:16 a.m. |
| Message ID | <differential-rev-PHID-DREV-t7nupojuptq4ngvo64wb-req@mercurial-scm.org> |
| Download | mbox | patch |
| Permalink | /patch/41409/ |
| State | Superseded |
| Headers | show |
Comments
kevincox added inline comments. kevincox accepted this revision. INLINE COMMENTS > filepatterns.rs:161 > } > - let mut res = b".*".to_vec(); > - res.extend(pattern); > - res > + [&b".*"[..], pattern].concat() > } I don't think you need the `&_[..]`. https://rust.godbolt.org/z/Wo-vza REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6765/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6765 To: valentin.gatienbaron, #hg-reviewers, Alphare, kevincox Cc: durin42, kevincox, mercurial-devel
valentin.gatienbaron added inline comments. INLINE COMMENTS > kevincox wrote in filepatterns.rs:161 > I don't think you need the `&_[..]`. https://rust.godbolt.org/z/Wo-vza Indeed, although it took a compiler upgrade. I don't really understand why the compiler accepts the change here but not in the other places. It seems a bit random to remove the borrow only here. What do you think about the next commit? It would remove the `&_[..]` more predictably. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6765/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6765 To: valentin.gatienbaron, #hg-reviewers, Alphare, kevincox Cc: durin42, kevincox, mercurial-devel
kevincox added inline comments. INLINE COMMENTS > valentin.gatienbaron wrote in filepatterns.rs:161 > Indeed, although it took a compiler upgrade. > I don't really understand why the compiler accepts the change here but not in the other places. It seems a bit random to remove the borrow only here. What do you think about the next commit? It would remove the `&_[..]` more predictably. For the other ones I think the problem is that it gets the element type from the first element as `&Vec<u8>`. I think a better approach then creating the wrapper function is doing `[vec.as_slice(), b"foobar"].concat()`. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6765/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6765 To: valentin.gatienbaron, #hg-reviewers, Alphare, kevincox Cc: durin42, kevincox, mercurial-devel
valentin.gatienbaron added inline comments. valentin.gatienbaron marked 2 inline comments as done. INLINE COMMENTS > kevincox wrote in filepatterns.rs:161 > For the other ones I think the problem is that it gets the element type from the first element as `&Vec<u8>`. I think a better approach then creating the wrapper function is doing `[vec.as_slice(), b"foobar"].concat()`. I see. So the reason why the code above was accepted is because `pattern` was already a `&[u8]`, and not a `&Vec<u8>`. Thanks! REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6765/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6765 To: valentin.gatienbaron, #hg-reviewers, Alphare, kevincox Cc: durin42, kevincox, mercurial-devel
Patch
diff --git a/rust/hg-core/src/filepatterns.rs b/rust/hg-core/src/filepatterns.rs --- a/rust/hg-core/src/filepatterns.rs +++ b/rust/hg-core/src/filepatterns.rs @@ -158,26 +158,20 @@ if pattern[0] == b'^' { return pattern.to_owned(); } - let mut res = b".*".to_vec(); - res.extend(pattern); - res + [&b".*"[..], pattern].concat() } PatternSyntax::Path | PatternSyntax::RelPath => { if pattern == b"." { return vec![]; } - let mut pattern = escape_pattern(pattern); - pattern.extend(b"(?:/|$)"); - pattern + [&escape_pattern(pattern), &b"(?:/|$)"[..]].concat() } PatternSyntax::RootFiles => { let mut res = if pattern == b"." { vec![] } else { // Pattern is a directory name. - let mut as_vec: Vec<u8> = escape_pattern(pattern); - as_vec.push(b'/'); - as_vec + [&escape_pattern(pattern), &b"/"[..]].concat() }; // Anything after the pattern must be a non-directory. @@ -185,24 +179,16 @@ res } PatternSyntax::RelGlob => { - let mut res: Vec<u8> = vec![]; let glob_re = glob_to_re(pattern); if let Some(rest) = glob_re.chop_prefix(b"[^/]*") { - res.extend(b".*"); - res.extend(rest); + [&b".*"[..], rest, globsuffix].concat() } else { - res.extend(b"(?:|.*/)"); - res.extend(glob_re); + [&b"(?:|.*/)"[..], &glob_re, globsuffix].concat() } - res.extend(globsuffix.iter()); - res } PatternSyntax::Glob | PatternSyntax::RootGlob => { - let mut res: Vec<u8> = vec![]; - res.extend(glob_to_re(pattern)); - res.extend(globsuffix.iter()); - res + [&glob_to_re(pattern), globsuffix].concat() } } }