ls: gnu color-clear-to-eol fix (#6507)

---------

Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>

src/uu/ls/src/colors.rs

@@ -28,7 +28,12 @@ impl<'a> StyleManager<'a> {
         }
     }
 
-    pub(crate) fn apply_style(&mut self, new_style: Option<&Style>, name: &str) -> String {
+    pub(crate) fn apply_style(
+        &mut self,
+        new_style: Option<&Style>,
+        name: &str,
+        wrap: bool,
+    ) -> String {
         let mut style_code = String::new();
         let mut force_suffix_reset: bool = false;
 
@@ -57,7 +62,17 @@ impl<'a> StyleManager<'a> {
             force_suffix_reset = true;
         }
 
-        format!("{}{}{}", style_code, name, self.reset(force_suffix_reset))
+        // we need this clear to eol code in some terminals, for instance if the
+        // text is in the last row of the terminal meaning the terminal need to
+        // scroll up in order to print new text in this situation if the clear
+        // to eol code is not present the background of the text would stretch
+        // till the end of line
+        let clear_to_eol = if wrap { "\x1b[K" } else { "" };
+
+        format!(
+            "{style_code}{name}{}{clear_to_eol}",
+            self.reset(force_suffix_reset),
+        )
     }
 
     /// Resets the current style and returns the default ANSI reset code to
@@ -110,20 +125,22 @@ impl<'a> StyleManager<'a> {
         path: &PathData,
         md_option: Option<&Metadata>,
         name: &str,
+        wrap: bool,
     ) -> String {
         let style = self
             .colors
             .style_for_path_with_metadata(&path.p_buf, md_option);
-        self.apply_style(style, name)
+        self.apply_style(style, name, wrap)
     }
 
     pub(crate) fn apply_style_based_on_dir_entry(
         &mut self,
         dir_entry: &DirEntry,
         name: &str,
+        wrap: bool,
     ) -> String {
         let style = self.colors.style_for(dir_entry);
-        self.apply_style(style, name)
+        self.apply_style(style, name, wrap)
     }
 }
 
@@ -137,12 +154,13 @@ pub(crate) fn color_name(
     style_manager: &mut StyleManager,
     out: &mut BufWriter<Stdout>,
     target_symlink: Option<&PathData>,
+    wrap: bool,
 ) -> 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 style_manager.apply_style_based_on_dir_entry(de, name);
+            return style_manager.apply_style_based_on_dir_entry(de, name, wrap);
         }
     }
 
@@ -152,11 +170,11 @@ pub(crate) fn color_name(
         // should not exit with an err, if we are unable to obtain the target_metadata
         let md_res = get_metadata_with_deref_opt(&target.p_buf, path.must_dereference);
         let md = md_res.or(path.p_buf.symlink_metadata());
-        style_manager.apply_style_based_on_metadata(path, md.ok().as_ref(), name)
+        style_manager.apply_style_based_on_metadata(path, md.ok().as_ref(), name, wrap)
     } 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());
-        style_manager.apply_style_based_on_metadata(path, md, name)
+        style_manager.apply_style_based_on_metadata(path, md, name, wrap)
     }
 }

src/uu/ls/src/ls.rs

@@ -929,8 +929,6 @@ impl Config {
 
         let width = match options.get_one::<String>(options::WIDTH) {
             Some(x) => parse_width(x)?,
-            None => match terminal_size::terminal_size() {
-                Some((width, _)) => width.0,
             None => match std::env::var_os("COLUMNS") {
                 Some(columns) => match columns.to_str().and_then(|s| s.parse().ok()) {
                     Some(columns) => columns,
@@ -942,6 +940,8 @@ impl Config {
                         DEFAULT_TERM_WIDTH
                     }
                 },
+                None => match terminal_size::terminal_size() {
+                    Some((width, _)) => width.0,
                     None => DEFAULT_TERM_WIDTH,
                 },
             },
@@ -2540,7 +2540,13 @@ fn display_items(
         let mut names_vec = Vec::new();
         for i in items {
             let more_info = display_additional_leading_info(i, &padding, config, out)?;
-            let cell = display_item_name(i, config, prefix_context, more_info, out, style_manager);
+            // it's okay to set current column to zero which is used to decide
+            // whether text will wrap or not, because when format is grid or
+            // column ls will try to place the item name in a new line if it
+            // wraps.
+            let cell =
+                display_item_name(i, config, prefix_context, more_info, out, style_manager, 0);
+
             names_vec.push(cell);
         }
 
@@ -2817,7 +2823,15 @@ fn display_item_long(
 
         write!(output_display, " {} ", display_date(md, config)).unwrap();
 
-        let item_name = display_item_name(item, config, None, String::new(), out, style_manager);
+        let item_name = display_item_name(
+            item,
+            config,
+            None,
+            String::new(),
+            out,
+            style_manager,
+            ansi_width(&output_display),
+        );
 
         let displayed_item = if quoted && !item_name.starts_with('\'') {
             format!(" {}", item_name)
@@ -2906,8 +2920,15 @@ fn display_item_long(
             write!(output_display, " {}", pad_right("?", padding.uname)).unwrap();
         }
 
-        let displayed_item =
+        let displayed_item = display_item_name(
-            display_item_name(item, config, None, String::new(), out, style_manager);
+            item,
+            config,
+            None,
+            String::new(),
+            out,
+            style_manager,
+            ansi_width(&output_display),
+        );
         let date_len = 12;
 
         write!(
@@ -3168,16 +3189,20 @@ fn display_item_name(
     more_info: String,
     out: &mut BufWriter<Stdout>,
     style_manager: &mut Option<StyleManager>,
+    current_column: usize,
 ) -> String {
     // This is our return value. We start by `&path.display_name` and modify it along the way.
     let mut name = escape_name(&path.display_name, &config.quoting_style);
 
+    let is_wrap =
+        |namelen: usize| config.width != 0 && current_column + namelen > config.width.into();
+
     if config.hyperlink {
         name = create_hyperlink(&name, path);
     }
 
     if let Some(style_manager) = style_manager {
-        name = color_name(&name, path, style_manager, out, None);
+        name = color_name(&name, path, style_manager, out, None, is_wrap(name.len()));
     }
 
     if config.format != Format::Long && !more_info.is_empty() {
@@ -3254,6 +3279,7 @@ fn display_item_name(
                             style_manager,
                             out,
                             Some(&target_data),
+                            is_wrap(name.len()),
                         ));
                     }
                 } else {

tests/by-util/test_ls.rs

@@ -4803,3 +4803,26 @@ fn test_ls_color_norm() {
         .succeeds()
         .stdout_contains(expected);
 }
+
+#[test]
+fn test_ls_color_clear_to_eol() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    // we create file with a long name
+    at.touch("zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.foo");
+    let result = scene
+        .ucmd()
+        .env("TERM", "xterm")
+        // set the columns to something small so that the text would wrap around
+        .env("COLUMNS", "80")
+        // set the background to green and text to red
+        .env("LS_COLORS", "*.foo=0;31;42")
+        .env("TIME_STYLE", "+T")
+        .arg("-og")
+        .arg("--color")
+        .arg("zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.foo")
+        .succeeds();
+    // check that the wrapped name contains clear to end of line code
+    // cspell:disable-next-line
+    result.stdout_contains("\x1b[0m\x1b[31;42mzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.foo\x1b[0m\x1b[K");
+}

util/build-gnu.sh

@@ -356,3 +356,13 @@ sed -i -E 's/(\^\[\[0m)+/\^\[\[0m/g' tests/ls/color-norm.sh
 # variable, but our ls seems to output them in a predefined order. Nevertheless,
 # the order doesn't matter, so it's okay.
 sed -i  's/44;37/37;44/' tests/ls/multihardlink.sh
+
+# Just like mentioned in the previous patch, GNU's ls output color codes in the
+# same way it is specified in the environment variable, but our ls emits them
+# differently. In this case, the color code is set to 0;31;42, and our ls would
+# ignore the 0; part. This would have been a bug if we output color codes
+# individually, for example, ^[[31^[[42 instead of ^[[31;42, but we don't do
+# that anywhere in our implementation, and it looks like GNU's ls also doesn't
+# do that. So, it's okay to ignore the zero.
+sed -i  "s/color_code='0;31;42'/color_code='31;42'/" tests/ls/color-clear-to-eol.sh
+