1
Fork 0
mirror of https://github.com/RGBCube/uutils-coreutils synced 2025-07-27 11:07:44 +00:00

Merge pull request #8271 from RenjiSann/quoting-simplify

uucore(quoting_style): Improve quoting style handling
This commit is contained in:
Sylvestre Ledru 2025-06-26 17:11:09 +02:00 committed by GitHub
commit 507e1cc09b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 108 additions and 145 deletions

View file

@ -8,7 +8,7 @@ use std::ffi::OsString;
use std::path::Path; use std::path::Path;
use uu_ls::{Config, Format, options}; use uu_ls::{Config, Format, options};
use uucore::error::UResult; use uucore::error::UResult;
use uucore::quoting_style::{Quotes, QuotingStyle}; use uucore::quoting_style::QuotingStyle;
#[uucore::main] #[uucore::main]
pub fn uumain(args: impl uucore::Args) -> UResult<()> { pub fn uumain(args: impl uucore::Args) -> UResult<()> {
@ -45,9 +45,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let mut config = Config::from(&matches)?; let mut config = Config::from(&matches)?;
if default_quoting_style { if default_quoting_style {
config.quoting_style = QuotingStyle::C { config.quoting_style = QuotingStyle::C_NO_QUOTES;
quotes: Quotes::None,
};
} }
if default_format_style { if default_format_style {
config.format = Format::Columns; config.format = Format::Columns;

View file

@ -61,9 +61,7 @@ use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR};
use uucore::libc::{dev_t, major, minor}; use uucore::libc::{dev_t, major, minor};
use uucore::line_ending::LineEnding; use uucore::line_ending::LineEnding;
use uucore::locale::{get_message, get_message_with_args}; use uucore::locale::{get_message, get_message_with_args};
use uucore::quoting_style::{ use uucore::quoting_style::{QuotingStyle, locale_aware_escape_dir_name, locale_aware_escape_name};
self, QuotingStyle, locale_aware_escape_dir_name, locale_aware_escape_name,
};
use uucore::{ use uucore::{
display::Quotable, display::Quotable,
error::{UError, UResult, set_exit_code}, error::{UError, UResult, set_exit_code},
@ -598,34 +596,15 @@ fn extract_hyperlink(options: &clap::ArgMatches) -> bool {
fn match_quoting_style_name(style: &str, show_control: bool) -> Option<QuotingStyle> { fn match_quoting_style_name(style: &str, show_control: bool) -> Option<QuotingStyle> {
match style { match style {
"literal" => Some(QuotingStyle::Literal { show_control }), "literal" => Some(QuotingStyle::Literal { show_control }),
"shell" => Some(QuotingStyle::Shell { "shell" => Some(QuotingStyle::SHELL),
escape: false, "shell-always" => Some(QuotingStyle::SHELL_QUOTE),
always_quote: false, "shell-escape" => Some(QuotingStyle::SHELL_ESCAPE),
show_control, "shell-escape-always" => Some(QuotingStyle::SHELL_ESCAPE_QUOTE),
}), "c" => Some(QuotingStyle::C_DOUBLE),
"shell-always" => Some(QuotingStyle::Shell { "escape" => Some(QuotingStyle::C_NO_QUOTES),
escape: false,
always_quote: true,
show_control,
}),
"shell-escape" => Some(QuotingStyle::Shell {
escape: true,
always_quote: false,
show_control,
}),
"shell-escape-always" => Some(QuotingStyle::Shell {
escape: true,
always_quote: true,
show_control,
}),
"c" => Some(QuotingStyle::C {
quotes: quoting_style::Quotes::Double,
}),
"escape" => Some(QuotingStyle::C {
quotes: quoting_style::Quotes::None,
}),
_ => None, _ => None,
} }
.map(|qs| qs.show_control(show_control))
} }
/// Extracts the quoting style to use based on the options provided. /// Extracts the quoting style to use based on the options provided.
@ -651,13 +630,9 @@ fn extract_quoting_style(options: &clap::ArgMatches, show_control: bool) -> Quot
} else if options.get_flag(options::quoting::LITERAL) { } else if options.get_flag(options::quoting::LITERAL) {
QuotingStyle::Literal { show_control } QuotingStyle::Literal { show_control }
} else if options.get_flag(options::quoting::ESCAPE) { } else if options.get_flag(options::quoting::ESCAPE) {
QuotingStyle::C { QuotingStyle::C_NO_QUOTES
quotes: quoting_style::Quotes::None,
}
} else if options.get_flag(options::quoting::C) { } else if options.get_flag(options::quoting::C) {
QuotingStyle::C { QuotingStyle::C_DOUBLE
quotes: quoting_style::Quotes::Double,
}
} else if options.get_flag(options::DIRED) { } else if options.get_flag(options::DIRED) {
QuotingStyle::Literal { show_control } QuotingStyle::Literal { show_control }
} else { } else {
@ -684,11 +659,7 @@ fn extract_quoting_style(options: &clap::ArgMatches, show_control: bool) -> Quot
// By default, `ls` uses Shell escape quoting style when writing to a terminal file // By default, `ls` uses Shell escape quoting style when writing to a terminal file
// descriptor and Literal otherwise. // descriptor and Literal otherwise.
if stdout().is_terminal() { if stdout().is_terminal() {
QuotingStyle::Shell { QuotingStyle::SHELL_ESCAPE.show_control(show_control)
escape: true,
always_quote: false,
show_control,
}
} else { } else {
QuotingStyle::Literal { show_control } QuotingStyle::Literal { show_control }
} }
@ -2010,7 +1981,7 @@ fn show_dir_name(
config: &Config, config: &Config,
) -> std::io::Result<()> { ) -> std::io::Result<()> {
let escaped_name = let escaped_name =
locale_aware_escape_dir_name(path_data.p_buf.as_os_str(), &config.quoting_style); locale_aware_escape_dir_name(path_data.p_buf.as_os_str(), config.quoting_style);
let name = if config.hyperlink && !config.dired { let name = if config.hyperlink && !config.dired {
create_hyperlink(&escaped_name, path_data) create_hyperlink(&escaped_name, path_data)
@ -2511,7 +2482,7 @@ fn display_items(
// option, print the security context to the left of the size column. // option, print the security context to the left of the size column.
let quoted = items.iter().any(|item| { let quoted = items.iter().any(|item| {
let name = locale_aware_escape_name(&item.display_name, &config.quoting_style); let name = locale_aware_escape_name(&item.display_name, config.quoting_style);
os_str_starts_with(&name, b"'") os_str_starts_with(&name, b"'")
}); });
@ -3175,7 +3146,7 @@ fn display_item_name(
current_column: LazyCell<usize, Box<dyn FnOnce() -> usize + '_>>, current_column: LazyCell<usize, Box<dyn FnOnce() -> usize + '_>>,
) -> OsString { ) -> OsString {
// This is our return value. We start by `&path.display_name` and modify it along the way. // This is our return value. We start by `&path.display_name` and modify it along the way.
let mut name = locale_aware_escape_name(&path.display_name, &config.quoting_style); let mut name = locale_aware_escape_name(&path.display_name, config.quoting_style);
let is_wrap = let is_wrap =
|namelen: usize| config.width != 0 && *current_column + namelen > config.width.into(); |namelen: usize| config.width != 0 && *current_column + namelen > config.width.into();
@ -3267,7 +3238,7 @@ fn display_item_name(
name.push(path.p_buf.read_link().unwrap()); name.push(path.p_buf.read_link().unwrap());
} else { } else {
name.push(color_name( name.push(color_name(
locale_aware_escape_name(target.as_os_str(), &config.quoting_style), locale_aware_escape_name(target.as_os_str(), config.quoting_style),
path, path,
style_manager, style_manager,
&mut state.out, &mut state.out,
@ -3280,7 +3251,7 @@ fn display_item_name(
// Apply the right quoting // Apply the right quoting
name.push(locale_aware_escape_name( name.push(locale_aware_escape_name(
target.as_os_str(), target.as_os_str(),
&config.quoting_style, config.quoting_style,
)); ));
} }
} }

View file

@ -8,7 +8,7 @@ use std::ffi::OsString;
use std::path::Path; use std::path::Path;
use uu_ls::{Config, Format, options}; use uu_ls::{Config, Format, options};
use uucore::error::UResult; use uucore::error::UResult;
use uucore::quoting_style::{Quotes, QuotingStyle}; use uucore::quoting_style::QuotingStyle;
#[uucore::main] #[uucore::main]
pub fn uumain(args: impl uucore::Args) -> UResult<()> { pub fn uumain(args: impl uucore::Args) -> UResult<()> {
@ -44,9 +44,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let mut config = Config::from(&matches)?; let mut config = Config::from(&matches)?;
if default_quoting_style { if default_quoting_style {
config.quoting_style = QuotingStyle::C { config.quoting_style = QuotingStyle::C_NO_QUOTES;
quotes: Quotes::None,
};
} }
if default_format_style { if default_format_style {
config.format = Format::Long; config.format = Format::Long;

View file

@ -127,17 +127,6 @@ mod options {
static ARG_FILES: &str = "files"; static ARG_FILES: &str = "files";
static STDIN_REPR: &str = "-"; static STDIN_REPR: &str = "-";
static QS_ESCAPE: &QuotingStyle = &QuotingStyle::Shell {
escape: true,
always_quote: false,
show_control: false,
};
static QS_QUOTE_ESCAPE: &QuotingStyle = &QuotingStyle::Shell {
escape: true,
always_quote: true,
show_control: false,
};
/// Supported inputs. /// Supported inputs.
#[derive(Debug)] #[derive(Debug)]
enum Inputs<'a> { enum Inputs<'a> {
@ -260,7 +249,8 @@ impl<'a> Input<'a> {
let path = path.as_os_str(); let path = path.as_os_str();
if path.to_string_lossy().contains('\n') { if path.to_string_lossy().contains('\n') {
Some(Cow::Owned(quoting_style::locale_aware_escape_name( Some(Cow::Owned(quoting_style::locale_aware_escape_name(
path, QS_ESCAPE, path,
QuotingStyle::SHELL_ESCAPE,
))) )))
} else { } else {
Some(Cow::Borrowed(path)) Some(Cow::Borrowed(path))
@ -761,7 +751,10 @@ fn files0_iter_file<'a>(path: &Path) -> UResult<impl Iterator<Item = InputIterIt
"wc-error-cannot-open-for-reading", "wc-error-cannot-open-for-reading",
HashMap::from([( HashMap::from([(
"path".to_string(), "path".to_string(),
quoting_style::locale_aware_escape_name(path.as_os_str(), QS_QUOTE_ESCAPE) quoting_style::locale_aware_escape_name(
path.as_os_str(),
QuotingStyle::SHELL_ESCAPE_QUOTE,
)
.into_string() .into_string()
.expect("All escaped names with the escaping option return valid strings."), .expect("All escaped names with the escaping option return valid strings."),
)]), )]),
@ -814,7 +807,7 @@ fn files0_iter<'a>(
} }
fn escape_name_wrapper(name: &OsStr) -> String { fn escape_name_wrapper(name: &OsStr) -> String {
quoting_style::locale_aware_escape_name(name, QS_ESCAPE) quoting_style::locale_aware_escape_name(name, QuotingStyle::SHELL_ESCAPE)
.into_string() .into_string()
.expect("All escaped names with the escaping option return valid strings.") .expect("All escaped names with the escaping option return valid strings.")
} }

View file

@ -8,7 +8,7 @@ use crate::format::spec::ArgumentLocation;
use crate::{ use crate::{
error::set_exit_code, error::set_exit_code,
parser::num_parser::{ExtendedParser, ExtendedParserError}, parser::num_parser::{ExtendedParser, ExtendedParserError},
quoting_style::{Quotes, QuotingStyle, locale_aware_escape_name}, quoting_style::{QuotingStyle, locale_aware_escape_name},
show_error, show_warning, show_error, show_warning,
}; };
use os_display::Quotable; use os_display::Quotable;
@ -153,12 +153,7 @@ fn extract_value<T: Default>(p: Result<T, ExtendedParserError<'_, T>>, input: &s
Ok(v) => v, Ok(v) => v,
Err(e) => { Err(e) => {
set_exit_code(1); set_exit_code(1);
let input = locale_aware_escape_name( let input = locale_aware_escape_name(OsStr::new(input), QuotingStyle::C_NO_QUOTES);
OsStr::new(input),
&QuotingStyle::C {
quotes: Quotes::None,
},
);
match e { match e {
ExtendedParserError::Overflow(v) => { ExtendedParserError::Overflow(v) => {
show_error!("{}: Numerical result out of range", input.quote()); show_error!("{}: Numerical result out of range", input.quote());

View file

@ -404,11 +404,7 @@ impl Spec {
Self::QuotedString { position } => { Self::QuotedString { position } => {
let s = locale_aware_escape_name( let s = locale_aware_escape_name(
args.next_string(position).as_ref(), args.next_string(position).as_ref(),
&QuotingStyle::Shell { QuotingStyle::SHELL_ESCAPE,
escape: true,
always_quote: false,
show_control: false,
},
); );
#[cfg(unix)] #[cfg(unix)]
let bytes = std::os::unix::ffi::OsStringExt::into_vec(s); let bytes = std::os::unix::ffi::OsStringExt::into_vec(s);

View file

@ -52,6 +52,60 @@ pub enum QuotingStyle {
}, },
} }
/// Provide sane defaults for quoting styles.
impl QuotingStyle {
pub const SHELL: Self = Self::Shell {
escape: false,
always_quote: false,
show_control: false,
};
pub const SHELL_ESCAPE: Self = Self::Shell {
escape: true,
always_quote: false,
show_control: false,
};
pub const SHELL_QUOTE: Self = Self::Shell {
escape: false,
always_quote: true,
show_control: false,
};
pub const SHELL_ESCAPE_QUOTE: Self = Self::Shell {
escape: true,
always_quote: true,
show_control: false,
};
pub const C_NO_QUOTES: Self = Self::C {
quotes: Quotes::None,
};
pub const C_DOUBLE: Self = Self::C {
quotes: Quotes::Double,
};
/// Set the `show_control` field of the quoting style.
/// Note: this is a no-op for the `C` variant.
pub fn show_control(self, show_control: bool) -> Self {
use QuotingStyle::*;
match self {
Shell {
escape,
always_quote,
..
} => Shell {
escape,
always_quote,
show_control,
},
Literal { .. } => Literal { show_control },
C { .. } => self,
}
}
}
/// Common interface of quoting mechanisms. /// Common interface of quoting mechanisms.
trait Quoter { trait Quoter {
/// Push a valid character. /// Push a valid character.
@ -92,7 +146,7 @@ pub enum Quotes {
/// is meant for ls' directory name display. /// is meant for ls' directory name display.
fn escape_name_inner( fn escape_name_inner(
name: &[u8], name: &[u8],
style: &QuotingStyle, style: QuotingStyle,
dirname: bool, dirname: bool,
encoding: UEncoding, encoding: UEncoding,
) -> Vec<u8> { ) -> Vec<u8> {
@ -103,14 +157,14 @@ fn escape_name_inner(
let mut quoter: Box<dyn Quoter> = match style { let mut quoter: Box<dyn Quoter> = match style {
QuotingStyle::Literal { .. } => Box::new(LiteralQuoter::new(name.len())), QuotingStyle::Literal { .. } => Box::new(LiteralQuoter::new(name.len())),
QuotingStyle::C { quotes } => Box::new(CQuoter::new(*quotes, dirname, name.len())), QuotingStyle::C { quotes } => Box::new(CQuoter::new(quotes, dirname, name.len())),
QuotingStyle::Shell { QuotingStyle::Shell {
escape: true, escape: true,
always_quote, always_quote,
.. ..
} => Box::new(EscapedShellQuoter::new( } => Box::new(EscapedShellQuoter::new(
name, name,
*always_quote, always_quote,
dirname, dirname,
name.len(), name.len(),
)), )),
@ -120,8 +174,8 @@ fn escape_name_inner(
show_control, show_control,
} => Box::new(NonEscapedShellQuoter::new( } => Box::new(NonEscapedShellQuoter::new(
name, name,
*show_control, show_control,
*always_quote, always_quote,
dirname, dirname,
name.len(), name.len(),
)), )),
@ -149,28 +203,28 @@ fn escape_name_inner(
} }
/// Escape a filename with respect to the given style. /// Escape a filename with respect to the given style.
pub fn escape_name(name: &OsStr, style: &QuotingStyle, encoding: UEncoding) -> OsString { pub fn escape_name(name: &OsStr, style: QuotingStyle, encoding: UEncoding) -> OsString {
let name = crate::os_str_as_bytes_lossy(name); let name = crate::os_str_as_bytes_lossy(name);
crate::os_string_from_vec(escape_name_inner(&name, style, false, encoding)) crate::os_string_from_vec(escape_name_inner(&name, style, false, encoding))
.expect("all byte sequences should be valid for platform, or already replaced in name") .expect("all byte sequences should be valid for platform, or already replaced in name")
} }
/// Retrieve the encoding from the locale and pass it to `escape_name`. /// Retrieve the encoding from the locale and pass it to `escape_name`.
pub fn locale_aware_escape_name(name: &OsStr, style: &QuotingStyle) -> OsString { pub fn locale_aware_escape_name(name: &OsStr, style: QuotingStyle) -> OsString {
escape_name(name, style, i18n::get_locale_encoding()) escape_name(name, style, i18n::get_locale_encoding())
} }
/// Escape a directory name with respect to the given style. /// Escape a directory name with respect to the given style.
/// This is mainly meant to be used for ls' directory name printing and is not /// This is mainly meant to be used for ls' directory name printing and is not
/// likely to be used elsewhere. /// likely to be used elsewhere.
pub fn escape_dir_name(dir_name: &OsStr, style: &QuotingStyle, encoding: UEncoding) -> OsString { pub fn escape_dir_name(dir_name: &OsStr, style: QuotingStyle, encoding: UEncoding) -> OsString {
let name = crate::os_str_as_bytes_lossy(dir_name); let name = crate::os_str_as_bytes_lossy(dir_name);
crate::os_string_from_vec(escape_name_inner(&name, style, true, encoding)) crate::os_string_from_vec(escape_name_inner(&name, style, true, encoding))
.expect("all byte sequences should be valid for platform, or already replaced in name") .expect("all byte sequences should be valid for platform, or already replaced in name")
} }
/// Retrieve the encoding from the locale and pass it to `escape_dir_name`. /// Retrieve the encoding from the locale and pass it to `escape_dir_name`.
pub fn locale_aware_escape_dir_name(name: &OsStr, style: &QuotingStyle) -> OsString { pub fn locale_aware_escape_dir_name(name: &OsStr, style: QuotingStyle) -> OsString {
escape_dir_name(name, style, i18n::get_locale_encoding()) escape_dir_name(name, style, i18n::get_locale_encoding())
} }
@ -225,49 +279,21 @@ mod tests {
show_control: false, show_control: false,
}, },
"literal-show" => QuotingStyle::Literal { show_control: true }, "literal-show" => QuotingStyle::Literal { show_control: true },
"escape" => QuotingStyle::C { "escape" => QuotingStyle::C_NO_QUOTES,
quotes: Quotes::None, "c" => QuotingStyle::C_DOUBLE,
}, "shell" => QuotingStyle::SHELL,
"c" => QuotingStyle::C { "shell-show" => QuotingStyle::SHELL.show_control(true),
quotes: Quotes::Double, "shell-always" => QuotingStyle::SHELL_QUOTE,
}, "shell-always-show" => QuotingStyle::SHELL_QUOTE.show_control(true),
"shell" => QuotingStyle::Shell { "shell-escape" => QuotingStyle::SHELL_ESCAPE,
escape: false, "shell-escape-always" => QuotingStyle::SHELL_ESCAPE_QUOTE,
always_quote: false,
show_control: false,
},
"shell-show" => QuotingStyle::Shell {
escape: false,
always_quote: false,
show_control: true,
},
"shell-always" => QuotingStyle::Shell {
escape: false,
always_quote: true,
show_control: false,
},
"shell-always-show" => QuotingStyle::Shell {
escape: false,
always_quote: true,
show_control: true,
},
"shell-escape" => QuotingStyle::Shell {
escape: true,
always_quote: false,
show_control: false,
},
"shell-escape-always" => QuotingStyle::Shell {
escape: true,
always_quote: true,
show_control: false,
},
_ => panic!("Invalid name!"), _ => panic!("Invalid name!"),
} }
} }
fn check_names_inner<T>(encoding: UEncoding, name: &[u8], map: &[(T, &str)]) -> Vec<Vec<u8>> { fn check_names_inner<T>(encoding: UEncoding, name: &[u8], map: &[(T, &str)]) -> Vec<Vec<u8>> {
map.iter() map.iter()
.map(|(_, style)| escape_name_inner(name, &get_style(style), false, encoding)) .map(|(_, style)| escape_name_inner(name, get_style(style), false, encoding))
.collect() .collect()
} }
@ -1034,30 +1060,16 @@ mod tests {
#[test] #[test]
fn test_quoting_style_display() { fn test_quoting_style_display() {
let style = QuotingStyle::Shell { let style = QuotingStyle::SHELL_ESCAPE;
escape: true,
always_quote: false,
show_control: false,
};
assert_eq!(format!("{style}"), "shell-escape"); assert_eq!(format!("{style}"), "shell-escape");
let style = QuotingStyle::Shell { let style = QuotingStyle::SHELL_QUOTE;
escape: false,
always_quote: true,
show_control: false,
};
assert_eq!(format!("{style}"), "shell-always-quote"); assert_eq!(format!("{style}"), "shell-always-quote");
let style = QuotingStyle::Shell { let style = QuotingStyle::SHELL.show_control(true);
escape: false,
always_quote: false,
show_control: true,
};
assert_eq!(format!("{style}"), "shell-show-control"); assert_eq!(format!("{style}"), "shell-show-control");
let style = QuotingStyle::C { let style = QuotingStyle::C_DOUBLE;
quotes: Quotes::Double,
};
assert_eq!(format!("{style}"), "C"); assert_eq!(format!("{style}"), "C");
let style = QuotingStyle::Literal { let style = QuotingStyle::Literal {