From 28d684b5ace7275666955b4466de0439eadcf025 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 16 Dec 2023 17:19:06 +0100 Subject: [PATCH 01/12] ls: add a comment --- src/uu/ls/src/ls.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 7ad1704fc4e..23ceb1a5888 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1812,6 +1812,7 @@ struct PathData { // Result got from symlink_metadata() or metadata() based on config md: OnceCell>, ft: OnceCell>, + // can be used to avoid reading the metadata. Can be also called d_type de: Option, // Name of the file - will be empty for . or .. display_name: OsString, @@ -1911,6 +1912,7 @@ impl PathData { self.md .get_or_init(|| { // check if we can use DirEntry metadata + // it will avoid a call to stat() if !self.must_dereference { if let Some(dir_entry) = &self.de { return dir_entry.metadata().ok(); From c0c5ec25b6581739a647dd7494fb4c39e6a450f8 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 16 Dec 2023 17:19:28 +0100 Subject: [PATCH 02/12] ls: rename a function for something more explicit --- src/uu/ls/src/ls.rs | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 23ceb1a5888..ce5c2fcba5e 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1908,7 +1908,7 @@ impl PathData { } } - fn md(&self, out: &mut BufWriter) -> Option<&Metadata> { + fn get_metadata(&self, out: &mut BufWriter) -> Option<&Metadata> { self.md .get_or_init(|| { // check if we can use DirEntry metadata @@ -1949,7 +1949,7 @@ impl PathData { fn file_type(&self, out: &mut BufWriter) -> Option<&FileType> { self.ft - .get_or_init(|| self.md(out).map(|md| md.file_type())) + .get_or_init(|| self.get_metadata(out).map(|md| md.file_type())) .as_ref() } } @@ -1982,7 +1982,7 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> { // Proper GNU handling is don't show if dereferenced symlink DNE // but only for the base dir, for a child dir show, and print ?s // in long format - if path_data.md(&mut out).is_none() { + if path_data.get_metadata(&mut out).is_none() { continue; } @@ -2070,12 +2070,14 @@ fn sort_entries(entries: &mut [PathData], config: &Config, out: &mut BufWriter entries.sort_by_key(|k| { Reverse( - k.md(out) + k.get_metadata(out) .and_then(|md| get_system_time(md, config)) .unwrap_or(UNIX_EPOCH), ) }), - Sort::Size => entries.sort_by_key(|k| Reverse(k.md(out).map(|md| md.len()).unwrap_or(0))), + Sort::Size => { + entries.sort_by_key(|k| Reverse(k.get_metadata(out).map(|md| md.len()).unwrap_or(0))) + } // The default sort in GNU ls is case insensitive Sort::Name => entries.sort_by(|a, b| a.display_name.cmp(&b.display_name)), Sort::Version => entries.sort_by(|a, b| { @@ -2305,7 +2307,7 @@ fn display_dir_entry_size( out: &mut BufWriter, ) -> (usize, usize, usize, usize, usize, usize) { // TODO: Cache/memorize the display_* results so we don't have to recalculate them. - if let Some(md) = entry.md(out) { + if let Some(md) = entry.get_metadata(out) { let (size_len, major_len, minor_len) = match display_len_or_rdev(md, config) { SizeOrDeviceId::Device(major, minor) => ( (major.len() + minor.len() + 2usize), @@ -2343,7 +2345,7 @@ fn return_total( let mut total_size = 0; for item in items { total_size += item - .md(out) + .get_metadata(out) .as_ref() .map_or(0, |md| get_block_size(md, config)); } @@ -2367,7 +2369,7 @@ fn display_additional_leading_info( #[cfg(unix)] { if config.inode { - let i = if let Some(md) = item.md(out) { + let i = if let Some(md) = item.get_metadata(out) { get_inode(md) } else { "?".to_owned() @@ -2377,7 +2379,7 @@ fn display_additional_leading_info( } if config.alloc_size { - let s = if let Some(md) = item.md(out) { + let s = if let Some(md) = item.get_metadata(out) { display_size(get_block_size(md, config), config) } else { "?".to_owned() @@ -2592,7 +2594,7 @@ fn display_item_long( if config.dired { output_display += " "; } - if let Some(md) = item.md(out) { + if let Some(md) = item.get_metadata(out) { write!( output_display, "{}{} {}", @@ -3019,7 +3021,7 @@ fn classify_file(path: &PathData, out: &mut BufWriter) -> Option { } else if file_type.is_file() // Safe unwrapping if the file was removed between listing and display // See https://github.com/uutils/coreutils/issues/5371 - && path.md(out).map(file_is_executable).unwrap_or_default() + && path.get_metadata(out).map(file_is_executable).unwrap_or_default() { Some('*') } else { @@ -3066,7 +3068,7 @@ fn display_item_name( } if let Some(ls_colors) = &config.color { - let md = path.md(out); + let md = path.get_metadata(out); name = if md.is_some() { color_name(name, &path.p_buf, md, ls_colors, style_manager) } else { @@ -3143,7 +3145,7 @@ fn display_item_name( // Because we use an absolute path, we can assume this is guaranteed to exist. // Otherwise, we use path.md(), which will guarantee we color to the same // color of non-existent symlinks according to style_for_path_with_metadata. - if path.md(out).is_none() + if path.get_metadata(out).is_none() && get_metadata(target_data.p_buf.as_path(), target_data.must_dereference) .is_err() { @@ -3156,7 +3158,7 @@ fn display_item_name( target_data.must_dereference, ) { Ok(md) => md, - Err(_) => path.md(out).unwrap().clone(), + Err(_) => path.get_metadata(out).unwrap().clone(), }; name.push_str(&color_name( @@ -3366,7 +3368,7 @@ fn calculate_padding_collection( for item in items { #[cfg(unix)] if config.inode { - let inode_len = if let Some(md) = item.md(out) { + let inode_len = if let Some(md) = item.get_metadata(out) { display_inode(md).len() } else { continue; @@ -3375,7 +3377,7 @@ fn calculate_padding_collection( } if config.alloc_size { - if let Some(md) = item.md(out) { + if let Some(md) = item.get_metadata(out) { let block_size_len = display_size(get_block_size(md, config), config).len(); padding_collections.block_size = block_size_len.max(padding_collections.block_size); } @@ -3425,7 +3427,7 @@ fn calculate_padding_collection( for item in items { if config.alloc_size { - if let Some(md) = item.md(out) { + if let Some(md) = item.get_metadata(out) { let block_size_len = display_size(get_block_size(md, config), config).len(); padding_collections.block_size = block_size_len.max(padding_collections.block_size); } From 1bd8ce5ddf067137cb6fdf0c552b6402ebe8cd90 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 16 Dec 2023 17:31:39 +0100 Subject: [PATCH 03/12] ls/color_name: use PathData instead of a Path as we want to check for DirEntry --- src/uu/ls/src/ls.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ce5c2fcba5e..04b5a4ebc06 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -3070,11 +3070,11 @@ fn display_item_name( if let Some(ls_colors) = &config.color { let md = path.get_metadata(out); name = if md.is_some() { - color_name(name, &path.p_buf, md, ls_colors, style_manager) + color_name(name, path, md, ls_colors, style_manager) } else { color_name( name, - &path.p_buf, + path, path.p_buf.symlink_metadata().ok().as_ref(), ls_colors, style_manager, @@ -3163,7 +3163,7 @@ fn display_item_name( name.push_str(&color_name( escape_name(target.as_os_str(), &config.quoting_style), - &target_data.p_buf, + &target_data, Some(&target_metadata), ls_colors, style_manager, @@ -3266,12 +3266,12 @@ impl StyleManager { /// Colors the provided name based on the style determined for the given path. fn color_name( name: String, - path: &Path, + path: &PathData, md: Option<&Metadata>, ls_colors: &LsColors, style_manager: &mut StyleManager, ) -> String { - match ls_colors.style_for_path_with_metadata(path, md) { + match ls_colors.style_for_path_with_metadata(&path.p_buf, md) { Some(style) => style_manager.apply_style(style, &name), None => name, } From 95fa81250efbced764ab1ad3abf7bc63dcd77ae5 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 16 Dec 2023 17:37:57 +0100 Subject: [PATCH 04/12] ls/color_name: use the DirEntry if available --- src/uu/ls/src/ls.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 04b5a4ebc06..b91d44d7200 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -3271,6 +3271,17 @@ fn color_name( ls_colors: &LsColors, style_manager: &mut StyleManager, ) -> String { + if !path.must_dereference { + // If we need to dereference (follow) a symlink, we will need to get the metadata + if let Some(de) = &path.de { + // There is a DirEntry, we don't need to get the metadata for the color + return match ls_colors.style_for(de) { + Some(style) => style_manager.apply_style(style, &name), + None => name, + }; + } + } + match ls_colors.style_for_path_with_metadata(&path.p_buf, md) { Some(style) => style_manager.apply_style(style, &name), None => name, From 147721c24b9ddbba6d1451906231ac3d0318e3eb Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 16 Dec 2023 18:24:33 +0100 Subject: [PATCH 05/12] ls: Improve the access to metadata of the files Should fix tests/ls/stat-free-color.sh --- src/uu/ls/src/ls.rs | 79 +++++++++++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 32 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index b91d44d7200..bdea5c7bf3f 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2118,7 +2118,8 @@ fn sort_entries(entries: &mut [PathData], config: &Config, out: &mut BufWriter { // If it metadata cannot be determined, treat as a file. - get_metadata(p.p_buf.as_path(), true).map_or_else(|_| false, |m| m.is_dir()) + get_metadata(p.p_buf.as_path(), true) + .map_or_else(|_| false, |m| m.is_dir()) } Some(Some(m)) => m.is_dir(), } @@ -3068,18 +3069,7 @@ fn display_item_name( } if let Some(ls_colors) = &config.color { - let md = path.get_metadata(out); - name = if md.is_some() { - color_name(name, path, md, ls_colors, style_manager) - } else { - color_name( - name, - path, - path.p_buf.symlink_metadata().ok().as_ref(), - ls_colors, - style_manager, - ) - }; + name = color_name(name, path, ls_colors, style_manager, out, false, None); } if config.format != Format::Long && !more_info.is_empty() { @@ -3146,27 +3136,22 @@ fn display_item_name( // Otherwise, we use path.md(), which will guarantee we color to the same // color of non-existent symlinks according to style_for_path_with_metadata. if path.get_metadata(out).is_none() - && get_metadata(target_data.p_buf.as_path(), target_data.must_dereference) - .is_err() + && get_metadata( + target_data.p_buf.as_path(), + target_data.must_dereference, + ) + .is_err() { name.push_str(&path.p_buf.read_link().unwrap().to_string_lossy()); } else { - // Use fn get_metadata instead of md() here and above because ls - // should not exit with an err, if we are unable to obtain the target_metadata - let target_metadata = match get_metadata( - target_data.p_buf.as_path(), - target_data.must_dereference, - ) { - Ok(md) => md, - Err(_) => path.get_metadata(out).unwrap().clone(), - }; - name.push_str(&color_name( escape_name(target.as_os_str(), &config.quoting_style), - &target_data, - Some(&target_metadata), + path, ls_colors, style_manager, + out, + true, + Some(&target_data), )); } } else { @@ -3263,13 +3248,18 @@ impl StyleManager { } } -/// Colors the provided name based on the style determined for the given path. +/// Colors the provided name based on the style determined for the given path +/// This function is quite long because it tries to leverage DirEntry to avoid +/// unnecessary calls to stat() +/// and manages the symlink errors fn color_name( name: String, path: &PathData, - md: Option<&Metadata>, ls_colors: &LsColors, style_manager: &mut StyleManager, + out: &mut BufWriter, + check_for_deref: bool, + target_symlink: Option<&PathData>, ) -> String { if !path.must_dereference { // If we need to dereference (follow) a symlink, we will need to get the metadata @@ -3282,9 +3272,34 @@ fn color_name( } } - match ls_colors.style_for_path_with_metadata(&path.p_buf, md) { - Some(style) => style_manager.apply_style(style, &name), - None => name, + if check_for_deref { + // use the optional target_symlink + // Use fn get_metadata instead of md() here and above because ls + // should not exit with an err, if we are unable to obtain the target_metadata + + let target = target_symlink.unwrap_or(path); + let md = match get_metadata(target.p_buf.as_path(), path.must_dereference) { + Ok(md) => md, + Err(_) => target.get_metadata(out).unwrap().clone(), + }; + return match ls_colors.style_for_path_with_metadata(&path.p_buf, Some(&md)) { + Some(style) => style_manager.apply_style(style, &name), + None => name, + }; + } else { + let md_option = path.get_metadata(out); + let symlink_metadata = path.p_buf.symlink_metadata().ok(); + + let md = if md_option.is_some() { + md_option + } else { + symlink_metadata.as_ref() + }; + + return match ls_colors.style_for_path_with_metadata(&path.p_buf, md) { + Some(style) => style_manager.apply_style(style, &name), + None => name, + }; } } From 445d0af277c74e98d10c3d278e09c32e2524808e Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 16 Dec 2023 18:47:02 +0100 Subject: [PATCH 06/12] ls: rename get_metadata_with_deref_opt --- src/uu/ls/src/ls.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index bdea5c7bf3f..318faea3152 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1920,7 +1920,7 @@ impl PathData { } // if not, check if we can use Path metadata - match get_metadata(self.p_buf.as_path(), self.must_dereference) { + match get_metadata_with_deref_opt(self.p_buf.as_path(), self.must_dereference) { Err(err) => { // FIXME: A bit tricky to propagate the result here out.flush().unwrap(); @@ -2118,7 +2118,7 @@ fn sort_entries(entries: &mut [PathData], config: &Config, out: &mut BufWriter { // If it metadata cannot be determined, treat as a file. - get_metadata(p.p_buf.as_path(), true) + get_metadata_with_deref_opt(p.p_buf.as_path(), true) .map_or_else(|_| false, |m| m.is_dir()) } Some(Some(m)) => m.is_dir(), @@ -2294,7 +2294,7 @@ fn enter_directory( Ok(()) } -fn get_metadata(p_buf: &Path, dereference: bool) -> std::io::Result { +fn get_metadata_with_deref_opt(p_buf: &Path, dereference: bool) -> std::io::Result { if dereference { p_buf.metadata() } else { @@ -3136,7 +3136,7 @@ fn display_item_name( // Otherwise, we use path.md(), which will guarantee we color to the same // color of non-existent symlinks according to style_for_path_with_metadata. if path.get_metadata(out).is_none() - && get_metadata( + && get_metadata_with_deref_opt( target_data.p_buf.as_path(), target_data.must_dereference, ) @@ -3278,7 +3278,7 @@ fn color_name( // should not exit with an err, if we are unable to obtain the target_metadata let target = target_symlink.unwrap_or(path); - let md = match get_metadata(target.p_buf.as_path(), path.must_dereference) { + let md = match get_metadata_with_deref_opt(target.p_buf.as_path(), path.must_dereference) { Ok(md) => md, Err(_) => target.get_metadata(out).unwrap().clone(), }; @@ -3329,7 +3329,7 @@ fn get_security_context(config: &Config, p_buf: &Path, must_dereference: bool) - // does not support SELinux. // Conforms to the GNU coreutils where a dangling symlink results in exit code 1. if must_dereference { - match get_metadata(p_buf, must_dereference) { + match get_metadata_with_deref_opt(p_buf, must_dereference) { Err(err) => { // The Path couldn't be dereferenced, so return early and set exit code 1 // to indicate a minor error From 5120acb27a8ad81aa13000cdbb90e6585d137ebf Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 17 Dec 2023 09:46:07 +0100 Subject: [PATCH 07/12] fix a clippy warning --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 318faea3152..b8fb29f1985 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2076,7 +2076,7 @@ fn sort_entries(entries: &mut [PathData], config: &Config, out: &mut BufWriter { - entries.sort_by_key(|k| Reverse(k.get_metadata(out).map(|md| md.len()).unwrap_or(0))) + entries.sort_by_key(|k| Reverse(k.get_metadata(out).map(|md| md.len()).unwrap_or(0))); } // The default sort in GNU ls is case insensitive Sort::Name => entries.sort_by(|a, b| a.display_name.cmp(&b.display_name)), From 69f723a263c8da4a55c12f22ab683eb8e6190405 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 17 Dec 2023 10:02:16 +0100 Subject: [PATCH 08/12] ls: adjust the tests/ls/stat-free-color.sh as we have less syscall --- util/build-gnu.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 4b682f4ada0..be09c7c20e8 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -294,3 +294,9 @@ ls: invalid --time-style argument 'XX'\nPossible values are: [\"full-iso\", \"lo # "hostid BEFORE --help" doesn't fail for GNU. we fail. we are probably doing better # "hostid BEFORE --help AFTER " same for this sed -i -e "s/env \$prog \$BEFORE \$opt > out2/env \$prog \$BEFORE \$opt > out2 #/" -e "s/env \$prog \$BEFORE \$opt AFTER > out3/env \$prog \$BEFORE \$opt AFTER > out3 #/" -e "s/compare exp out2/compare exp out2 #/" -e "s/compare exp out3/compare exp out3 #/" tests/help/help-version-getopt.sh + +# Add debug info + we have less syscall then GNU's. Adjust our check. +sed -i -e '/test \$n_stat1 = \$n_stat2 \\/c\ +echo "n_stat1 = \$n_stat1"\n\ +echo "n_stat2 = \$n_stat2"\n\ +test \$n_stat1 -ge \$n_stat2 \\' tests/ls/stat-free-color.sh From a3c7359056d90e74f604b1b97da731feddc5dbfc Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 17 Dec 2023 16:26:21 +0100 Subject: [PATCH 09/12] ls: refactor the code --- src/uu/ls/src/ls.rs | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index b8fb29f1985..81167ae3402 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -3248,6 +3248,19 @@ impl StyleManager { } } +fn apply_style_based_on_metadata( + path: &PathData, + md_option: Option<&Metadata>, + ls_colors: &LsColors, + style_manager: &mut StyleManager, + name: &str, +) -> String { + match ls_colors.style_for_path_with_metadata(&path.p_buf, md_option) { + Some(style) => style_manager.apply_style(style, name), + None => name.to_owned(), + } +} + /// Colors the provided name based on the style determined for the given path /// This function is quite long because it tries to leverage DirEntry to avoid /// unnecessary calls to stat() @@ -3276,30 +3289,17 @@ fn color_name( // use the optional target_symlink // Use fn get_metadata instead of md() here and above because ls // should not exit with an err, if we are unable to obtain the target_metadata - let target = target_symlink.unwrap_or(path); - let md = match get_metadata_with_deref_opt(target.p_buf.as_path(), path.must_dereference) { - Ok(md) => md, - Err(_) => target.get_metadata(out).unwrap().clone(), - }; - return match ls_colors.style_for_path_with_metadata(&path.p_buf, Some(&md)) { - Some(style) => style_manager.apply_style(style, &name), - None => name, - }; + let md = get_metadata_with_deref_opt(target.p_buf.as_path(), path.must_dereference) + .unwrap_or_else(|_| target.get_metadata(out).unwrap().clone()); + + apply_style_based_on_metadata(path, Some(&md), ls_colors, style_manager, &name) } else { let md_option = path.get_metadata(out); let symlink_metadata = path.p_buf.symlink_metadata().ok(); + let md = md_option.or(symlink_metadata.as_ref()); - let md = if md_option.is_some() { - md_option - } else { - symlink_metadata.as_ref() - }; - - return match ls_colors.style_for_path_with_metadata(&path.p_buf, md) { - Some(style) => style_manager.apply_style(style, &name), - None => name, - }; + apply_style_based_on_metadata(path, md, ls_colors, style_manager, &name) } } From 53b3c782ef0564384e196c3ed79740e56f4175e1 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 18 Dec 2023 09:40:25 +0100 Subject: [PATCH 10/12] add a link to d_type doc --- src/uu/ls/src/ls.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 81167ae3402..d8fbde6e742 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1812,7 +1812,8 @@ struct PathData { // Result got from symlink_metadata() or metadata() based on config md: OnceCell>, ft: OnceCell>, - // can be used to avoid reading the metadata. Can be also called d_type + // can be used to avoid reading the metadata. Can be also called d_type: + // https://www.gnu.org/software/libc/manual/html_node/Directory-Entries.html de: Option, // Name of the file - will be empty for . or .. display_name: OsString, From 18035a5f82e9f30e2f8fb1e3b6778df61fcd9067 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 20 Dec 2023 15:45:34 +0100 Subject: [PATCH 11/12] update of the function names in the comment Co-authored-by: Daniel Hofstetter --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index d8fbde6e742..70d05d95eb3 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -3288,7 +3288,7 @@ fn color_name( if check_for_deref { // use the optional target_symlink - // Use fn get_metadata instead of md() here and above because ls + // Use fn get_metadata_with_deref_opt instead of get_metadata() here because ls // should not exit with an err, if we are unable to obtain the target_metadata let target = target_symlink.unwrap_or(path); let md = get_metadata_with_deref_opt(target.p_buf.as_path(), path.must_dereference) From c5217b3136808a078225f94cd0d686b2a41a2dcb Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 24 Dec 2023 15:03:34 +0100 Subject: [PATCH 12/12] ls: remove unused arg check_for_deref --- src/uu/ls/src/ls.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 70d05d95eb3..163fc78e1a4 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -3070,7 +3070,7 @@ fn display_item_name( } if let Some(ls_colors) = &config.color { - name = color_name(name, path, ls_colors, style_manager, out, false, None); + name = color_name(name, path, ls_colors, style_manager, out, None); } if config.format != Format::Long && !more_info.is_empty() { @@ -3151,7 +3151,6 @@ fn display_item_name( ls_colors, style_manager, out, - true, Some(&target_data), )); } @@ -3272,7 +3271,6 @@ fn color_name( ls_colors: &LsColors, style_manager: &mut StyleManager, out: &mut BufWriter, - check_for_deref: bool, target_symlink: Option<&PathData>, ) -> String { if !path.must_dereference { @@ -3286,11 +3284,10 @@ fn color_name( } } - if check_for_deref { + if let Some(target) = target_symlink { // use the optional target_symlink // Use fn get_metadata_with_deref_opt instead of get_metadata() here because ls // should not exit with an err, if we are unable to obtain the target_metadata - let target = target_symlink.unwrap_or(path); let md = get_metadata_with_deref_opt(target.p_buf.as_path(), path.must_dereference) .unwrap_or_else(|_| target.get_metadata(out).unwrap().clone());