From ec1781b021190914b88ee649724456a8cb47db37 Mon Sep 17 00:00:00 2001 From: Dorian Peron Date: Thu, 26 Jun 2025 16:04:40 +0200 Subject: [PATCH] quoting_style: introduce sane defaults for quoting styles to avoid duplicate code --- src/uu/dir/src/dir.rs | 6 +- src/uu/ls/src/ls.rs | 61 ++------ src/uu/vdir/src/vdir.rs | 6 +- src/uu/wc/src/wc.rs | 25 ++-- .../src/lib/features/format/argument.rs | 9 +- src/uucore/src/lib/features/format/spec.rs | 6 +- .../src/lib/features/quoting_style/mod.rs | 140 ++++++++++-------- 7 files changed, 108 insertions(+), 145 deletions(-) diff --git a/src/uu/dir/src/dir.rs b/src/uu/dir/src/dir.rs index 0a8a71c22..9f12a3d56 100644 --- a/src/uu/dir/src/dir.rs +++ b/src/uu/dir/src/dir.rs @@ -8,7 +8,7 @@ use std::ffi::OsString; use std::path::Path; use uu_ls::{Config, Format, options}; use uucore::error::UResult; -use uucore::quoting_style::{Quotes, QuotingStyle}; +use uucore::quoting_style::QuotingStyle; #[uucore::main] 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)?; if default_quoting_style { - config.quoting_style = QuotingStyle::C { - quotes: Quotes::None, - }; + config.quoting_style = QuotingStyle::C_NO_QUOTES; } if default_format_style { config.format = Format::Columns; diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 9a9e35e00..7ab752a15 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -61,9 +61,7 @@ use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR}; use uucore::libc::{dev_t, major, minor}; use uucore::line_ending::LineEnding; use uucore::locale::{get_message, get_message_with_args}; -use uucore::quoting_style::{ - self, QuotingStyle, locale_aware_escape_dir_name, locale_aware_escape_name, -}; +use uucore::quoting_style::{QuotingStyle, locale_aware_escape_dir_name, locale_aware_escape_name}; use uucore::{ display::Quotable, 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 { match style { "literal" => Some(QuotingStyle::Literal { show_control }), - "shell" => Some(QuotingStyle::Shell { - escape: false, - always_quote: false, - show_control, - }), - "shell-always" => Some(QuotingStyle::Shell { - 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, - }), + "shell" => Some(QuotingStyle::SHELL), + "shell-always" => Some(QuotingStyle::SHELL_QUOTE), + "shell-escape" => Some(QuotingStyle::SHELL_ESCAPE), + "shell-escape-always" => Some(QuotingStyle::SHELL_ESCAPE_QUOTE), + "c" => Some(QuotingStyle::C_DOUBLE), + "escape" => Some(QuotingStyle::C_NO_QUOTES), _ => None, } + .map(|qs| qs.show_control(show_control)) } /// 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) { QuotingStyle::Literal { show_control } } else if options.get_flag(options::quoting::ESCAPE) { - QuotingStyle::C { - quotes: quoting_style::Quotes::None, - } + QuotingStyle::C_NO_QUOTES } else if options.get_flag(options::quoting::C) { - QuotingStyle::C { - quotes: quoting_style::Quotes::Double, - } + QuotingStyle::C_DOUBLE } else if options.get_flag(options::DIRED) { QuotingStyle::Literal { show_control } } 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 // descriptor and Literal otherwise. if stdout().is_terminal() { - QuotingStyle::Shell { - escape: true, - always_quote: false, - show_control, - } + QuotingStyle::SHELL_ESCAPE.show_control(show_control) } else { QuotingStyle::Literal { show_control } } @@ -2010,7 +1981,7 @@ fn show_dir_name( config: &Config, ) -> std::io::Result<()> { 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 { 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. 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"'") }); @@ -3175,7 +3146,7 @@ fn display_item_name( current_column: LazyCell usize + '_>>, ) -> OsString { // 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 = |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()); } else { 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, style_manager, &mut state.out, @@ -3280,7 +3251,7 @@ fn display_item_name( // Apply the right quoting name.push(locale_aware_escape_name( target.as_os_str(), - &config.quoting_style, + config.quoting_style, )); } } diff --git a/src/uu/vdir/src/vdir.rs b/src/uu/vdir/src/vdir.rs index fbb8943ca..f63b6c30e 100644 --- a/src/uu/vdir/src/vdir.rs +++ b/src/uu/vdir/src/vdir.rs @@ -8,7 +8,7 @@ use std::ffi::OsString; use std::path::Path; use uu_ls::{Config, Format, options}; use uucore::error::UResult; -use uucore::quoting_style::{Quotes, QuotingStyle}; +use uucore::quoting_style::QuotingStyle; #[uucore::main] 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)?; if default_quoting_style { - config.quoting_style = QuotingStyle::C { - quotes: Quotes::None, - }; + config.quoting_style = QuotingStyle::C_NO_QUOTES; } if default_format_style { config.format = Format::Long; diff --git a/src/uu/wc/src/wc.rs b/src/uu/wc/src/wc.rs index c00b09bbf..ad78c5ea3 100644 --- a/src/uu/wc/src/wc.rs +++ b/src/uu/wc/src/wc.rs @@ -127,17 +127,6 @@ mod options { static ARG_FILES: &str = "files"; 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. #[derive(Debug)] enum Inputs<'a> { @@ -260,7 +249,8 @@ impl<'a> Input<'a> { let path = path.as_os_str(); if path.to_string_lossy().contains('\n') { Some(Cow::Owned(quoting_style::locale_aware_escape_name( - path, QS_ESCAPE, + path, + QuotingStyle::SHELL_ESCAPE, ))) } else { Some(Cow::Borrowed(path)) @@ -761,9 +751,12 @@ fn files0_iter_file<'a>(path: &Path) -> UResult( } 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() .expect("All escaped names with the escaping option return valid strings.") } diff --git a/src/uucore/src/lib/features/format/argument.rs b/src/uucore/src/lib/features/format/argument.rs index 349527db7..c555e151f 100644 --- a/src/uucore/src/lib/features/format/argument.rs +++ b/src/uucore/src/lib/features/format/argument.rs @@ -8,7 +8,7 @@ use crate::format::spec::ArgumentLocation; use crate::{ error::set_exit_code, parser::num_parser::{ExtendedParser, ExtendedParserError}, - quoting_style::{Quotes, QuotingStyle, locale_aware_escape_name}, + quoting_style::{QuotingStyle, locale_aware_escape_name}, show_error, show_warning, }; use os_display::Quotable; @@ -153,12 +153,7 @@ fn extract_value(p: Result>, input: &s Ok(v) => v, Err(e) => { set_exit_code(1); - let input = locale_aware_escape_name( - OsStr::new(input), - &QuotingStyle::C { - quotes: Quotes::None, - }, - ); + let input = locale_aware_escape_name(OsStr::new(input), QuotingStyle::C_NO_QUOTES); match e { ExtendedParserError::Overflow(v) => { show_error!("{}: Numerical result out of range", input.quote()); diff --git a/src/uucore/src/lib/features/format/spec.rs b/src/uucore/src/lib/features/format/spec.rs index 3cffc08bc..b34efd1d3 100644 --- a/src/uucore/src/lib/features/format/spec.rs +++ b/src/uucore/src/lib/features/format/spec.rs @@ -404,11 +404,7 @@ impl Spec { Self::QuotedString { position } => { let s = locale_aware_escape_name( args.next_string(position).as_ref(), - &QuotingStyle::Shell { - escape: true, - always_quote: false, - show_control: false, - }, + QuotingStyle::SHELL_ESCAPE, ); #[cfg(unix)] let bytes = std::os::unix::ffi::OsStringExt::into_vec(s); diff --git a/src/uucore/src/lib/features/quoting_style/mod.rs b/src/uucore/src/lib/features/quoting_style/mod.rs index e5edc8fe9..9613e579d 100644 --- a/src/uucore/src/lib/features/quoting_style/mod.rs +++ b/src/uucore/src/lib/features/quoting_style/mod.rs @@ -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. trait Quoter { /// Push a valid character. @@ -92,7 +146,7 @@ pub enum Quotes { /// is meant for ls' directory name display. fn escape_name_inner( name: &[u8], - style: &QuotingStyle, + style: QuotingStyle, dirname: bool, encoding: UEncoding, ) -> Vec { @@ -103,14 +157,14 @@ fn escape_name_inner( let mut quoter: Box = match style { 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 { escape: true, always_quote, .. } => Box::new(EscapedShellQuoter::new( name, - *always_quote, + always_quote, dirname, name.len(), )), @@ -120,8 +174,8 @@ fn escape_name_inner( show_control, } => Box::new(NonEscapedShellQuoter::new( name, - *show_control, - *always_quote, + show_control, + always_quote, dirname, name.len(), )), @@ -149,28 +203,28 @@ fn escape_name_inner( } /// 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); 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") } /// 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 a directory name with respect to the given style. /// This is mainly meant to be used for ls' directory name printing and is not /// 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); 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") } /// 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()) } @@ -225,49 +279,21 @@ mod tests { show_control: false, }, "literal-show" => QuotingStyle::Literal { show_control: true }, - "escape" => QuotingStyle::C { - quotes: Quotes::None, - }, - "c" => QuotingStyle::C { - quotes: Quotes::Double, - }, - "shell" => QuotingStyle::Shell { - escape: false, - 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, - }, + "escape" => QuotingStyle::C_NO_QUOTES, + "c" => QuotingStyle::C_DOUBLE, + "shell" => QuotingStyle::SHELL, + "shell-show" => QuotingStyle::SHELL.show_control(true), + "shell-always" => QuotingStyle::SHELL_QUOTE, + "shell-always-show" => QuotingStyle::SHELL_QUOTE.show_control(true), + "shell-escape" => QuotingStyle::SHELL_ESCAPE, + "shell-escape-always" => QuotingStyle::SHELL_ESCAPE_QUOTE, _ => panic!("Invalid name!"), } } fn check_names_inner(encoding: UEncoding, name: &[u8], map: &[(T, &str)]) -> Vec> { 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() } @@ -1034,30 +1060,16 @@ mod tests { #[test] fn test_quoting_style_display() { - let style = QuotingStyle::Shell { - escape: true, - always_quote: false, - show_control: false, - }; + let style = QuotingStyle::SHELL_ESCAPE; assert_eq!(format!("{style}"), "shell-escape"); - let style = QuotingStyle::Shell { - escape: false, - always_quote: true, - show_control: false, - }; + let style = QuotingStyle::SHELL_QUOTE; assert_eq!(format!("{style}"), "shell-always-quote"); - let style = QuotingStyle::Shell { - escape: false, - always_quote: false, - show_control: true, - }; + let style = QuotingStyle::SHELL.show_control(true); assert_eq!(format!("{style}"), "shell-show-control"); - let style = QuotingStyle::C { - quotes: Quotes::Double, - }; + let style = QuotingStyle::C_DOUBLE; assert_eq!(format!("{style}"), "C"); let style = QuotingStyle::Literal {