fix: -t with -m sums timestamps instead of taking the newest - #612
Merged
Conversation
The windows metadata time is a FILETIME (100ns ticks since 1601), not a unix epoch, so `-m` panics in get_pretty_file_modified_time. That is pre-existing and unrelated to this fix. Also spells 'filetimes' as two words so the typos check passes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
-t(group by file type) combined with-m(show filetimes) adds the timestampstogether, so every group shows a date a century in the future.
Three
.logfiles with mtimes 2020-01-01, 2021-06-15 and 2022-03-10:The same tree without
-talready prints 2020, 2021 and 2022 correctly, which isthe reference the grouped view should agree with.
Cause
When
by_filetimeis set,node.sizeholds a timestamp rather than a size, andget_all_file_typesalready knows that in two places:The per-extension accumulator in
build_by_all_file_typeswas not given the sametreatment, so it keeps doing
*cumulative_size += node.sizeand sums epochs.The fix
Pass
by_filetimedown and take the max instead of the sum in that one branch,matching what the surrounding code already does. Size mode is untouched.
Verification
test_show_files_by_type_with_filetimebuilds the three files above and assertsthe rendered date.
Reverting only the branch back to
+=fails it, and the failure output is thesymptom itself:
I kept a test here rather than going test-free as on #609: this one needs real
files on disk, since
build_by_all_file_typesonly counts nodes wherenode.name.is_file(), so it is not the kind of one-line config change thatreads correct on sight. The timestamps are set at midday UTC so the assertion
holds regardless of the runner's timezone.
cargo testis 31 + 8 + 32 + 4 passed, 0 failed.cargo fmt --checkclean.cargo clippyreports the same two pre-existing "empty line after doc comment"warnings before and after, in files I did not touch.
Disclosure: written with AI assistance (Claude Code). I built binaries from master and from the patched tree and produced the before and after above by running them, and ran the mutation check myself.