ls:fix ls dired reports #10527

mattsu2020 · 2026-01-28T10:51:20Z

Summary

Fix ls --dired (Emacs dired) position reporting and refactor the offset bookkeeping to be simpler/safer.

Details

Return (displayed name + dired length) from display_item_name to avoid duplicated length logic and fix quoting/symlink-related misreporting.

Track positions using a running line_offset in DiredOutput, removing previous-line offset helpers and unused parameters.

Disable the build-gnu sed hack for ls stat-failed line numbers

related
#10248

…handling This commit refactors the `display_item_name` function to return a `DisplayItemName` struct containing both the displayed name and its dired length, rather than just the OsString. This change simplifies the code by: 1. Eliminating the need to call `dired_name_len()` separately after getting the displayed name 2. Reducing redundant length calculations in the dired position handling 3. Making the code more maintainable by encapsulating related data together The change also fixes an issue where dired positions were being calculated incorrectly for symlink names that required quoting, ensuring proper highlighting in dired mode.

Removed unnecessary parameter from dired::calculate_and_update_positions function call, making the code cleaner and more maintainable.

Replace complex offset calculation logic with a dedicated line_offset field in DiredOutput. This simplifies position tracking by maintaining a running total of line lengths instead of repeatedly calculating offsets from previous positions. The change improves code clarity and reduces potential for off-by-one errors in dired position calculations.

Removed unnecessary line length calculation before calling dired::calculate_dired, as the function doesn't use this parameter. The line_len variable was calculated but never used within the dired::calculate_dired function call.

src/uu/ls/src/dired.rs

github-actions · 2026-01-28T11:00:48Z

GNU testsuite comparison:

GNU test failed: tests/ls/stat-failed. tests/ls/stat-failed is passing on 'main'. Maybe you have to rebase?

… function Removed the `get_offset_from_previous_line()` helper function and replaced its usage with direct access to `dired.line_offset`. This simplifies the code by eliminating an unnecessary abstraction layer while maintaining the same functionality. The change affects multiple functions that calculate byte positions for DIRED output formatting.

github-actions · 2026-01-28T11:49:55Z

GNU testsuite comparison:

GNU test failed: tests/ls/stat-failed. tests/ls/stat-failed is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

Reverts a sed command that was adjusting line numbers in the ls stat-failed test, likely due to changes in test output or structure that made the line number adjustment unnecessary or incorrect.

github-actions · 2026-01-28T12:14:53Z

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

sylvestre · 2026-01-29T06:56:56Z

src/uu/ls/src/dired.rs

        print_positions("//DIRED//", &dired.dired_positions);
    }
-    if config.recursive {
+    if !dired.subdired_positions.is_empty() {


Are you sure about this change? It seems that it is changing the behavior

SUBDIRED is needed not only for -R but also when multiple directories are listed. If we print it only when recursive is true, ls --dired dir1 dir2 will miss //SUBDIRED//, which changes behavior.

src/uu/ls/src/ls.rs

util/build-gnu.sh

…ithout -R This commit fixes an issue where `//SUBDIRED//` positions were not being printed when using `--dired` with multiple directory arguments but without the `-R` (recursive) flag. The SUBDIRED positions are needed whenever directory headings are printed, which occurs with multiple arguments regardless of whether recursion is enabled.

…alculation The commit renames the `DisplayItemName` struct to avoid conflicts and optimizes the `dired_name_len` calculation by only computing it when dired mode is enabled, improving performance for non-dired displays.

Removed commented sed command that was skipping a test for ls --dired functionality. The command was already commented out and no longer needed, cleaning up the build script.

github-actions · 2026-01-29T09:53:36Z

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/basenc/bounded-memory is now passing!

sylvestre · 2026-02-03T21:07:36Z

src/uu/ls/src/ls.rs

        );

-        let displayed_item = if quoted && !os_str_starts_with(&item_name, b"'") {
+        let mut dired_name_len = item_display.dired_name_len;


Line length calculation should only be done when config.dired is true

sylvestre · 2026-02-03T21:08:01Z

src/uu/ls/src/ls.rs

        output_display.extend(b" ");

        if config.dired {
+            let line_len =


Line length calculation duplicated - extract to helper function

Extracted the repeated line length calculation logic into a dedicated `calculate_line_len` function to improve code maintainability and reduce duplication. The function takes the output length, item length, and line ending as parameters and returns the total line length including the line ending characters.

This commit refactors the dired position calculation and item display logic in the ls utility to improve code clarity and reduce redundancy. The changes include: - Simplifying the dired position calculation by computing the displayed length directly - Moving the space insertion logic closer to the dired calculation - Reducing variable assignments and improving the flow of item display processing - Maintaining the same functionality while making the code more readable and maintainable The refactoring maintains backward compatibility and preserves all existing behavior while making the code easier to understand and modify.

sylvestre · 2026-02-04T12:27:25Z

src/uu/ls/src/ls.rs

-        let displayed_item = if quoted && !os_str_starts_with(&item_name, b"'") {
+        let needs_space = quoted && !os_str_starts_with(&item_display.displayed, b"'");
+
+        if config.dired {


Two nearly identical blocks (lines 3054-3067 and 3168-3180) in display_item_long(). Extract to helper function update_dired_for_item().

sylvestre · 2026-02-04T12:28:05Z

src/uu/ls/src/dired.rs

-
-    let start = offset_from_previous_line + DIRED_TRAILING_OFFSET + additional_offset;
+    let offset_from_previous_line = dired.line_offset + dired.padding;
+    let start = offset_from_previous_line + DIRED_TRAILING_OFFSET;


you removed the blank line support, no ?

No—the blank-line offset is still handled via dired.padding. The blank line is followed by dired.padding += 1, and calculate_subdired() uses dired.line_offset + dired.padding,
so the offset is preserved without additional_offset

Extracted the repeated dired position calculation and update logic into a new `update_dired_for_item` function to reduce code duplication and improve maintainability. The function consolidates the calculation of line length and subsequent dired position updates that were previously duplicated across multiple locations in the `display_item_long` function.

github-actions · 2026-02-04T12:51:57Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

github-actions · 2026-02-04T13:30:26Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/follow-name (passes in this run but fails in the 'main' branch)

mattsu2020 and others added 5 commits January 28, 2026 08:46

refactor: simplify dired position calculation call

4380495

Removed unnecessary parameter from dired::calculate_and_update_positions function call, making the code cleaner and more maintainable.

Merge branch 'uutils:main' into ls_dired

f77a365

sylvestre reviewed Jan 28, 2026

View reviewed changes

src/uu/ls/src/dired.rs Outdated Show resolved Hide resolved

mattsu2020 marked this pull request as draft January 28, 2026 10:58

chore: disable ls stat-failed test modification

cd1711a

Reverts a sed command that was adjusting line numbers in the ls stat-failed test, likely due to changes in test output or structure that made the line number adjustment unnecessary or incorrect.

mattsu2020 marked this pull request as ready for review January 28, 2026 12:39

sylvestre reviewed Jan 29, 2026

View reviewed changes

src/uu/ls/src/ls.rs Outdated Show resolved Hide resolved

sylvestre reviewed Jan 29, 2026

View reviewed changes

src/uu/ls/src/ls.rs Outdated Show resolved Hide resolved

sylvestre reviewed Jan 29, 2026

View reviewed changes

src/uu/ls/src/ls.rs Outdated Show resolved Hide resolved

sylvestre reviewed Jan 29, 2026

View reviewed changes

util/build-gnu.sh Outdated Show resolved Hide resolved

mattsu2020 added 3 commits January 29, 2026 18:31

chore: remove obsolete sed command for ls --dired test

e230576

Removed commented sed command that was skipping a test for ls --dired functionality. The command was already commented out and no longer needed, cleaning up the build script.

sylvestre reviewed Feb 3, 2026

View reviewed changes

mattsu2020 added 2 commits February 4, 2026 20:39

sylvestre reviewed Feb 4, 2026

View reviewed changes

Uh oh!

ls:fix ls dired reports #10527

Are you sure you want to change the base?

ls:fix ls dired reports #10527

Conversation

mattsu2020 commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Uh oh!

Uh oh!

github-actions bot commented Jan 28, 2026

Uh oh!

github-actions bot commented Jan 28, 2026

Uh oh!

github-actions bot commented Jan 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 4, 2026

Uh oh!

github-actions bot commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mattsu2020 commented Jan 28, 2026 •

edited

Loading