Skip to content

Commit

Permalink
ls: gnu color-norm test fix
Browse files Browse the repository at this point in the history
  • Loading branch information
matrixhead committed Jun 18, 2024
1 parent 94295f9 commit a968980
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 21 deletions.
5 changes: 3 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,10 @@ hostname = "0.4"
indicatif = "0.17.8"
itertools = "0.13.0"
libc = "0.2.153"
lscolors = { version = "0.16.0", default-features = false, features = [
# this is just for a draft pr
lscolors = { default-features = false, features = [
"gnu_legacy",
] }
], git = "https://github.com/matrixhead/lscolors" }
memchr = "2.7.2"
memmap2 = "0.9.4"
nix = { version = "0.28", default-features = false }
Expand Down
122 changes: 103 additions & 19 deletions src/uu/ls/src/ls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use clap::{
crate_version, Arg, ArgAction, Command,
};
use glob::{MatchOptions, Pattern};
use lscolors::{LsColors, Style};
use lscolors::{Indicator, LsColors, Style};

use ansi_width::ansi_width;
use std::{cell::OnceCell, num::IntErrorKind};
Expand Down Expand Up @@ -2071,6 +2071,15 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> {
sort_entries(&mut files, config, &mut out);
sort_entries(&mut dirs, config, &mut out);

if let Some(c) = &config.color {
// ls will try to write a reset before anything is written if normal
// color is given
if c.style_for_indicator(Indicator::Normal).is_some() {
let to_write = style_manager.reset(false);

Check warning on line 2078 in src/uu/ls/src/ls.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/ls/src/ls.rs#L2078

Added line #L2078 was not covered by tests
write!(out, "{}", to_write)?;
}

Check warning on line 2080 in src/uu/ls/src/ls.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/ls/src/ls.rs#L2080

Added line #L2080 was not covered by tests
}

display_items(&files, config, &mut out, &mut dired, &mut style_manager)?;

for (pos, path_data) in dirs.iter().enumerate() {
Expand Down Expand Up @@ -2520,6 +2529,14 @@ fn display_items(

let padding = calculate_padding_collection(items, config, out);

// we need to apply normal color to non filename output
if let Some(color) = &config.color {
if let Some(sty) = color.style_for_indicator(Indicator::Normal) {
let style_code = style_manager.get_style_code(sty);
write!(out, "{}", style_code).unwrap();
}

Check warning on line 2537 in src/uu/ls/src/ls.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/ls/src/ls.rs#L2535-L2537

Added lines #L2535 - L2537 were not covered by tests
}

let mut names_vec = Vec::new();
for i in items {
let more_info = display_additional_leading_info(i, &padding, config, out)?;
Expand Down Expand Up @@ -2694,6 +2711,14 @@ fn display_item_long(
quoted: bool,
) -> UResult<()> {
let mut output_display: String = String::new();

// apply normal color to non filename outputs
if let Some(color) = &config.color {
if let Some(sty) = color.style_for_indicator(Indicator::Normal) {
let style_code = style_manager.get_style_code(sty);
write!(output_display, "{}", style_code).unwrap();
}

Check warning on line 2720 in src/uu/ls/src/ls.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/ls/src/ls.rs#L2718-L2720

Added lines #L2718 - L2720 were not covered by tests
}
if config.dired {
output_display += " ";
}
Expand Down Expand Up @@ -3295,33 +3320,88 @@ fn create_hyperlink(name: &str, path: &PathData) -> String {
/// This because we need to check the previous value in case we don't need
/// the reset
struct StyleManager {
/// last style that is applied, if `None` that means reset is applied.
current_style: Option<Style>,
/// `true` if the initial reset is applied
initial_reset_is_done: bool,
}

impl StyleManager {
fn new() -> Self {
Self {
initial_reset_is_done: false,
current_style: None,
}
}

fn apply_style(&mut self, new_style: &Style, name: &str) -> String {
if let Some(current) = &self.current_style {
if *current == *new_style {
// Current style is the same as new style, apply without reset.
let mut style = new_style.to_nu_ansi_term_style();
style.prefix_with_reset = false;
return style.paint(name).to_string();
fn apply_style(
&mut self,
new_style: &Style,
name: &str,
ls_colors: &LsColors,
zeroed: bool,
) -> String {
let mut style_code = String::new();
let mut force_suffix_reset: bool = false;
// if reset is done we need to apply normal style before applying new style
if self.is_reset() {
if let Some(norm_sty) = ls_colors.style_for_indicator(Indicator::Normal) {
style_code.push_str(&self.get_style_code(norm_sty));

Check warning on line 3349 in src/uu/ls/src/ls.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/ls/src/ls.rs#L3349

Added line #L3349 was not covered by tests
}
}
// we only need to apply a new style if it's not the same as the current
// style for example if normal is the current style and a file with
// normal style is to be printed we could skip printing new color
// codes
if !self.is_current_style(new_style) {
style_code.push_str(&self.reset(false));
style_code.push_str(&self.get_style_code(new_style));
}
// lscolors might be set to zero value if it's set to zero value that
// means it should clear all style attributes including normal,so in the
// following code if it's not reset it means that it's running normal,
// then if zeroed is passed we should reset
else if !self.is_reset() && zeroed {
style_code.push_str(&self.reset(false));

Check warning on line 3365 in src/uu/ls/src/ls.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/ls/src/ls.rs#L3365

Added line #L3365 was not covered by tests
// even though this is an unnecessary reset for gnu compatibility we allow it here
force_suffix_reset = true;

Check warning on line 3367 in src/uu/ls/src/ls.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/ls/src/ls.rs#L3367

Added line #L3367 was not covered by tests
}
format!("{}{}{}", style_code, name, self.reset(force_suffix_reset))
}

// We are getting a new style, we need to reset it
self.current_style = Some(new_style.clone());
new_style
.to_nu_ansi_term_style()
.reset_before_style()
.paint(name)
.to_string()
/// Resets the current style and returns the default ANSI reset code to
/// reset all text formatting attributes. If `force` is true, the reset is
/// done even if the reset has been applied before.
fn reset(&mut self, force: bool) -> String {
// todo:
// We need to use style from `Indicator::Reset` but as of now ls colors
// uses a fallback mechanism and because of that if `Indicator::Reset`
// is not specified it would fallback to `Indicator::Normal` which seems
// to be non compatible with gnu
if self.current_style.is_some() | !self.initial_reset_is_done | force {
self.initial_reset_is_done = true;
self.current_style = None;
return "\x1b[0m".to_string();
}
String::new()
}

fn get_style_code(&mut self, new_style: &Style) -> String {
self.current_style = Some(*new_style);
let mut nu_a_style = new_style.to_nu_ansi_term_style();
nu_a_style.prefix_with_reset = false;
let mut ret = nu_a_style.paint("").to_string();
// remove the suffix reset
ret.truncate(ret.len() - 4);
ret
}

fn is_current_style(&mut self, new_style: &Style) -> bool {
matches!(&self.current_style,Some(style) if style == new_style )
}

fn is_reset(&mut self) -> bool {
self.current_style.is_none()
}
}

Expand All @@ -3332,8 +3412,10 @@ fn apply_style_based_on_metadata(
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),
let path_indicator = ls_colors.indicator_for_path_with_metadata(&path.p_buf, md_option);
let is_zeroed = ls_colors.zeroed_indicators.contains(&path_indicator);
match ls_colors.style_for_indicator(path_indicator) {
Some(style) => style_manager.apply_style(style, name, ls_colors, is_zeroed),
None => name.to_owned(),
}
}
Expand All @@ -3354,8 +3436,10 @@ fn color_name(
// 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),
let de_indicator = ls_colors.indicator_for(de);
let is_zeroed = ls_colors.zeroed_indicators.contains(&de_indicator);
return match ls_colors.style_for_indicator(de_indicator) {
Some(style) => style_manager.apply_style(style, &name, ls_colors, is_zeroed),
None => name,
};
}
Expand Down
8 changes: 8 additions & 0 deletions util/build-gnu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -343,3 +343,11 @@ test \$n_stat1 -ge \$n_stat2 \\' tests/ls/stat-free-color.sh

# no need to replicate this output with hashsum
sed -i -e "s|Try 'md5sum --help' for more information.\\\n||" tests/cksum/md5sum.pl

# Our ls command always outputs ANSI color codes prepended with a zero. However,
# in the case of GNU, it seems inconsistent. Nevertheless, it looks like it
# doesn't matter whether we prepend a zero or not.
sed -i -E '65,79{s/\^\[\[([1-9]m)/^[[0\1/g; s/\^\[\[m/^[[0m/g}' tests/ls/color-norm.sh
# It says in the test itself that having more than one reset is a bug, so we
# don't need to replicate that behavior.
sed -i -E '73,75{s/(\^\[\[0m)+/\^\[\[0m/g}' tests/ls/color-norm.sh

0 comments on commit a968980

Please sign in to comment.