Closed by commit rHG0d97bcb3cee9: rust-status: add util for listing a directory
(authored by Alphare).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs
Review".
REPOSITORY
rHG Mercurial
CHANGES
Alphare updated this revision to Diff 20183.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D7927?vs=19942=20183
BRANCH
default
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7927/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7927
kevincox added inline comments.
INLINE COMMENTS
> Alphare wrote in status.rs:76
> I didn't realize that `sort_unstable_by_key` needed to return a `T` and not a
> ``, that forces a `clone()`. I'm not sure that matters, we'll go with the
> shortest code and see if that shows up in profiling
Alphare added inline comments.
INLINE COMMENTS
> kevincox wrote in status.rs:76
> It would be more clear to do `.sort_by_key(|e| e.0)`
>
> And I don't think stability matters here so you could use
> `sort_unstable_by_key`.
I didn't realize that `sort_unstable_by_key` needed to return a `T`
kevincox added inline comments.
kevincox accepted this revision.
INLINE COMMENTS
> status.rs:76
> +
> +results.sort_by(|a, b| a.0.cmp());
> +Ok(results)
It would be more clear to do `.sort_by_key(|e| e.0)`
And I don't think stability matters here so you could use
Alphare added inline comments.
INLINE COMMENTS
> marmoute wrote in status.rs:74
> If I understand you code correctly you currently do:
>
> filenames = listdir()
> result = [(f, entry(f)) for f in filenames]
> result.sort()
>
> Would it make sense to do:
>
> filenames = listdir()
>
marmoute added inline comments.
INLINE COMMENTS
> Alphare wrote in status.rs:74
> I'm not entirely sure I understand your question.
If I understand you code correctly you currently do:
filenames = listdir()
result = [(f, entry(f)) for f in filenames]
result.sort()
Would it make sense to
Alphare updated this revision to Diff 19942.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D7927?vs=19416=19942
BRANCH
default
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7927/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7927
Alphare added inline comments.
Alphare marked an inline comment as done.
INLINE COMMENTS
> marmoute wrote in status.rs:74
> Silly question: would it make sense to sort the filename before the loop ? It
> might result is less bytes to move around since the entry will not be
> attached yet.
I'm
marmoute added a comment.
marmoute accepted this revision.
I am not a rust expert, but I don't see anything obviously wrong in there. I
make a couple of minor comments.
INLINE COMMENTS
> status.rs:68
> +if skip_dot_hg && filename.as_bytes() == b".hg" &&
> file_type.is_dir() {
> +
Alphare created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
I debated moving it to utils, but it is not used anywhere else for now, and
its skip behavior is pretty specific to status.
REPOSITORY
rHG
11 matches
Mail list logo
