diff --git a/src/uu/ls/src/colors.rs b/src/uu/ls/src/colors.rs index d41df2c090b..2a063d7550d 100644 --- a/src/uu/ls/src/colors.rs +++ b/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,20 @@ 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 green background of the text would stretch + // till the end of line + let clear_to_eol = if wrap { "\x1b[K" } else { "" }; + + format!( + "{}{}{}{}", + style_code, + name, + self.reset(force_suffix_reset), + clear_to_eol + ) } /// Resets the current style and returns the default ANSI reset code to @@ -110,20 +128,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 +157,13 @@ pub(crate) fn color_name( style_manager: &mut StyleManager, out: &mut BufWriter, 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 +173,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) } } diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index f3ec267ff6b..c25401ba22a 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -929,19 +929,19 @@ impl Config { let width = match options.get_one::(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, - None => { - show_error!( - "ignoring invalid width in environment variable COLUMNS: {}", - columns.quote() - ); - DEFAULT_TERM_WIDTH - } - }, + None => match std::env::var_os("COLUMNS") { + Some(columns) => match columns.to_str().and_then(|s| s.parse().ok()) { + Some(columns) => columns, + None => { + show_error!( + "ignoring invalid width in environment variable COLUMNS: {}", + columns.quote() + ); + 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 = - display_item_name(item, config, None, String::new(), out, style_manager); + let displayed_item = display_item_name( + 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, style_manager: &mut Option, + 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 { diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index cf73ba5d64f..bf4382e8edc 100644 --- a/tests/by-util/test_ls.rs +++ b/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"); +} diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 1049764642b..7631cf15216 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -355,4 +355,14 @@ sed -i -E 's/(\^\[\[0m)+/\^\[\[0m/g' tests/ls/color-norm.sh # GNU's ls seems to output color codes in the order given in the environment # 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 \ No newline at end of file +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 +