Skip to content

fix(ls): respect line_ending in display_grid when using --zero with column formats - #15505

Open
adamvalves wants to merge 3 commits into
uutils:mainfrom
adamvalves:fix/ls-zero-columns
Open

adamvalves wants to merge 3 commits into
uutils:mainfrom
adamvalves:fix/ls-zero-columns

Conversation

@adamvalves

Copy link
Copy Markdown

Fixes #14774

Summary of changes

  • Pass config.line_ending to display_grid in Format::Columns and Format::Across.
  • When LineEnding::Nul is active, format grid rows with \0 instead of \n.
  • Add regression tests covering --zero combined with -C and -x.

…olumn formats

When using --zero with -C or -x, lines were terminated by newline
instead of NUL because display_grid did not take line_ending into account.

Fixes uutils#14774
Comment thread src/uu/ls/src/display.rs Outdated
LineEnding::Newline => write!(out, "{grid}")?,
LineEnding::Nul => {
let grid_str = grid.to_string();
let nul_str = grid_str.replace('\n', "\0");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this also replaces newlines inside filenames, e.g. 1\ntest-zero with --zero -C (show-control-chars is on with --zero).
could you please only replace the row terminators?

Comment thread tests/by-util/test_ls.rs
for opt in ["-C", "-x"] {
scene
.ucmd()
.args(&["--zero", opt, "-w", "80"])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please also test with a smaller -w to get several rows, so we check every line ends with \0

Comment thread tests/by-util/test_ls.rs Outdated
.ucmd()
.args(&["--zero", opt, "-w", "80"])
.succeeds()
.stdout_only("0-test-zero\t2-test-zero\t3-test-zero\x00");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please add the same -C / -x check in the unix block below, with the 1\ntest-zero file

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Should be good now ight ?

This branch has not been deployed

No deployments
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 --zero ends the last line of -C / -x output with a newline instead of NUL

2 participants