Skip to content

fix(ls): respect sorting when grouping directories; disambiguate size sorting properly#12109

Merged
cakebaker merged 3 commits into
uutils:mainfrom
Alonely0:ls-group
May 5, 2026
Merged

fix(ls): respect sorting when grouping directories; disambiguate size sorting properly#12109
cakebaker merged 3 commits into
uutils:mainfrom
Alonely0:ls-group

Conversation

@Alonely0
Copy link
Copy Markdown
Contributor

@Alonely0 Alonely0 commented May 1, 2026

Fixes #11997.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

GNU testsuite comparison:

GNU test failed: tests/timeout/timeout-group. tests/timeout/timeout-group is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)

@cakebaker
Copy link
Copy Markdown
Contributor

Can you please add a test to ensure we don't regress in the future?

@xtqqczze

This comment was marked as resolved.

@Alonely0
Copy link
Copy Markdown
Contributor Author

Alonely0 commented May 1, 2026

Are the other instances in 7bfa92f OK?

It is because we do it for the first time on the buffer and only once; there stability doesn't matter. Let me add a test real quick.

@Alonely0

This comment was marked as outdated.

@Alonely0
Copy link
Copy Markdown
Contributor Author

Alonely0 commented May 1, 2026

That should be it. I had to also fix the size checking in order to get the new tests to pass (and that wasn't my fault LOL).

@Alonely0 Alonely0 changed the title fix(ls): respect sorting when grouping directories fix(ls): respect sorting when grouping directories; disambiguate size sorting properly May 1, 2026
@cakebaker cakebaker merged commit f7ba1f9 into uutils:main May 5, 2026
168 of 169 checks passed
@cakebaker
Copy link
Copy Markdown
Contributor

Thanks!

Comment thread src/uu/ls/src/ls.rs
Comment on lines 1475 to 1481
Sort::Time => entries.sort_unstable_by_key(|k| {
Reverse(
k.metadata()
.and_then(|md| metadata_get_time(md, config.time))
.unwrap_or(UNIX_EPOCH),
)
}),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So sorting by time will also put equivalently-timestamped entries in random order?

Copy link
Copy Markdown
Contributor Author

@Alonely0 Alonely0 May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also disambiguate those by name, yeah. Kinda surprising we previously left it in the order std gave it to us, although I have to say that this is essentially impossible outside fabricated scenarios or no{a, c, m}time filesystems. I'll look into it as well.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was raised on filesystems that only tracked one timestamp field and on software whose all files, at normal conditions, had a modification date/time of 1992-03-10 03:10:00. I know fabricated scenarios to be a fact of life, not a remote possibility.

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.

ls --group-directories-first results in an arbitrary ordering

4 participants