From 8b02c567fa634ae04eddb8a335e6ee864da5dd93 Mon Sep 17 00:00:00 2001 From: Roman Dolgov Date: Thu, 24 Sep 2026 16:12:20 +0400 Subject: [PATCH 1/2] ls: fix //DIRED-OPTIONS// printing the wrong quoting-style name --- src/uu/ls/src/dired.rs | 42 +++++++++++++++++++++++++++++++++++++++- tests/by-util/test_ls.rs | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/uu/ls/src/dired.rs b/src/uu/ls/src/dired.rs index ce96dbd3b82..6c8524e460f 100644 --- a/src/uu/ls/src/dired.rs +++ b/src/uu/ls/src/dired.rs @@ -35,10 +35,12 @@ /// Overall, the module ensures each entry in the DIRED output has the correct /// byte position, considering additional lines or padding affecting positions. /// +use crate::display::LocaleQuoting; use crate::{Config, LsError}; use std::fmt; use std::io::{BufWriter, Stdout, Write}; use uucore::error::UResult; +use uucore::quoting_style::{Quotes, QuotingStyle}; #[derive(Debug, Clone, PartialEq)] pub struct BytePosition { @@ -126,12 +128,50 @@ pub fn print_dired_output( writeln!( out, "//DIRED-OPTIONS// --quoting-style={}", - config.quoting_style + dired_quoting_style_name(config) ) .map_err(LsError::WriteError)?; Ok(()) } +/// The canonical `--quoting-style` name for the `//DIRED-OPTIONS//` trailer. +/// +/// `QuotingStyle`'s `Display` impl doesn't work here: several distinct style +/// names collapse onto the same `QuotingStyle` shape (`c`, `escape` and +/// `clocale` all reduce to `QuotingStyle::C`; `literal` and `locale` both +/// reduce to `QuotingStyle::Literal`), and it appends a `-show-control`/ +/// `-always-quote` suffix that GNU's `--dired` trailer never uses (those are +/// separate `ls` options, not part of the quoting-style name). +/// `config.locale_quoting` carries the extra bit needed to tell the +/// locale-aware pair apart from their non-locale counterparts. +fn dired_quoting_style_name(config: &Config) -> &'static str { + match config.locale_quoting { + Some(LocaleQuoting::Single) => return "locale", + Some(LocaleQuoting::Double) => return "clocale", + None => {} + } + match config.quoting_style { + QuotingStyle::Literal { .. } => "literal", + QuotingStyle::C { + quotes: Quotes::None, + } => "escape", + // `Quotes::Single` is never produced by ls's own option parsing + // (only reachable through `locale_quoting`, handled above), but + // `c` is the closest canonical name if that ever changes. + QuotingStyle::C { .. } => "c", + QuotingStyle::Shell { + escape, + always_quote, + .. + } => match (escape, always_quote) { + (false, false) => "shell", + (false, true) => "shell-always", + (true, false) => "shell-escape", + (true, true) => "shell-escape-always", + }, + } +} + /// Helper function to print positions with a given prefix. fn print_positions( out: &mut BufWriter, diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 6bb0aaa3672..d344716d1e1 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -5644,6 +5644,46 @@ fn test_ls_dired_implies_long() { .stdout_contains("//DIRED-OPTIONS// --quoting-style"); } +// Regression test for issue #14775: //DIRED-OPTIONS// must report the +// canonical name of the quoting style in effect, not a reconstruction from +// the internal QuotingStyle enum (several distinct style names collapse +// onto the same enum shape, and the enum's Display impl also appends a +// -show-control/-always-quote suffix that GNU's --dired trailer never uses). +#[test] +fn test_ls_dired_quoting_style_name() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("dir"); + + for style in [ + "literal", + "shell", + "shell-always", + "shell-escape", + "shell-escape-always", + "c", + "escape", + "locale", + "clocale", + ] { + scene + .ucmd() + .env("LC_ALL", "C") + .args(&["-l", "--dired", &format!("--quoting-style={style}"), "dir"]) + .succeeds() + .stdout_contains(format!("//DIRED-OPTIONS// --quoting-style={style}")); + } + + for (opt, style) in [("-N", "literal"), ("-Q", "c"), ("-b", "escape")] { + scene + .ucmd() + .env("LC_ALL", "C") + .args(&["-l", "--dired", opt, "dir"]) + .succeeds() + .stdout_contains(format!("//DIRED-OPTIONS// --quoting-style={style}")); + } +} + #[test] fn test_ls_dired_hyperlink() { // we will have link but not the DIRED output From ee81d2e755ec850acb2b0ad47af77c2365b32302 Mon Sep 17 00:00:00 2001 From: Roman Dolgov Date: Mon, 28 Sep 2026 01:49:21 +0400 Subject: [PATCH 2/2] shorten comments per review feedback --- src/uu/ls/src/dired.rs | 10 +--------- tests/by-util/test_ls.rs | 6 +----- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/uu/ls/src/dired.rs b/src/uu/ls/src/dired.rs index 6c8524e460f..b2b1e878a42 100644 --- a/src/uu/ls/src/dired.rs +++ b/src/uu/ls/src/dired.rs @@ -135,15 +135,7 @@ pub fn print_dired_output( } /// The canonical `--quoting-style` name for the `//DIRED-OPTIONS//` trailer. -/// -/// `QuotingStyle`'s `Display` impl doesn't work here: several distinct style -/// names collapse onto the same `QuotingStyle` shape (`c`, `escape` and -/// `clocale` all reduce to `QuotingStyle::C`; `literal` and `locale` both -/// reduce to `QuotingStyle::Literal`), and it appends a `-show-control`/ -/// `-always-quote` suffix that GNU's `--dired` trailer never uses (those are -/// separate `ls` options, not part of the quoting-style name). -/// `config.locale_quoting` carries the extra bit needed to tell the -/// locale-aware pair apart from their non-locale counterparts. +/// `QuotingStyle`'s `Display` impl collapses distinct style names together and adds suffixes GNU's trailer never uses, so this maps the config directly instead. fn dired_quoting_style_name(config: &Config) -> &'static str { match config.locale_quoting { Some(LocaleQuoting::Single) => return "locale", diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index d344716d1e1..9775afd9fc5 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -5644,11 +5644,7 @@ fn test_ls_dired_implies_long() { .stdout_contains("//DIRED-OPTIONS// --quoting-style"); } -// Regression test for issue #14775: //DIRED-OPTIONS// must report the -// canonical name of the quoting style in effect, not a reconstruction from -// the internal QuotingStyle enum (several distinct style names collapse -// onto the same enum shape, and the enum's Display impl also appends a -// -show-control/-always-quote suffix that GNU's --dired trailer never uses). +// Regression test for issue #14775. #[test] fn test_ls_dired_quoting_style_name() { let scene = TestScenario::new(util_name!());