Skip to content

ls: ls -lF symlink target indicators #11554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sylvestre merged 9 commits into from
May 28, 2026
+253 −81
Merged

ls: ls -lF symlink target indicators #11554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
982364e
ls: ls -lF symlink target indicator
joknarf Mar 30, 2026
f48941b
tests/ls: test for ls -lF symlink indicator
joknarf Mar 31, 2026
43b3834
fix(ls): take care of performance regressions
joknarf Mar 31, 2026
fabaf36
test(ls): add -lp slash indicator regression for symlink targets
sylvestre Apr 16, 2026
f5f3eae
test(ls): add --file-type exec/dir symlink target indicator
joknarf Apr 20, 2026
cf29c00
fix(ls): ls -lp symlink target w/o trailing slash
joknarf Apr 20, 2026
517f814
fix(ls): LS_COLORS mi= or=
joknarf Apr 20, 2026
039b19c
Merge pull request #1 from joknarf/classify_fix
joknarf May 27, 2026
89454cf
Merge branch 'main' into classify_target
joknarf May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 15 additions & 21 deletions src/uu/ls/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ pub struct Config {
// Dir and vdir needs access to this field
pub quoting_style: QuotingStyle,
pub(crate) locale_quoting: Option<LocaleQuoting>,
pub(crate) indicator_style: IndicatorStyle,
pub(crate) indicator_style: Option<IndicatorStyle>,
pub(crate) time_format_recent: String, // Time format for recent dates
pub(crate) time_format_older: Option<String>, // Time format for older dates (optional, if not present, time_format_recent is used)
pub(crate) context: bool,
Expand Down Expand Up @@ -598,34 +598,28 @@ fn extract_quoting_style(
/// # Returns
///
/// An [`IndicatorStyle`] variant representing the indicator style to use.
fn extract_indicator_style(options: &clap::ArgMatches) -> IndicatorStyle {
fn extract_indicator_style(options: &clap::ArgMatches) -> Option<IndicatorStyle> {
if let Some(field) = options.get_one::<String>(options::INDICATOR_STYLE) {
match field.as_str() {
"none" => IndicatorStyle::None,
"file-type" => IndicatorStyle::FileType,
"classify" => IndicatorStyle::Classify,
"slash" => IndicatorStyle::Slash,
&_ => IndicatorStyle::None,
"none" => None,
"file-type" => Some(IndicatorStyle::FileType),
"classify" => Some(IndicatorStyle::Classify),
"slash" => Some(IndicatorStyle::Slash),
&_ => None,
}
} else if let Some(field) = options.get_one::<String>(options::indicator_style::CLASSIFY) {
match field.as_str() {
"never" | "no" | "none" => IndicatorStyle::None,
"always" | "yes" | "force" => IndicatorStyle::Classify,
"auto" | "tty" | "if-tty" => {
if stdout().is_terminal() {
IndicatorStyle::Classify
} else {
IndicatorStyle::None
}
}
&_ => IndicatorStyle::None,
"never" | "no" | "none" => None,
"always" | "yes" | "force" => Some(IndicatorStyle::Classify),
"auto" | "tty" | "if-tty" => stdout().is_terminal().then_some(IndicatorStyle::Classify),
&_ => None,
}
} else if options.get_flag(options::indicator_style::SLASH) {
IndicatorStyle::Slash
Some(IndicatorStyle::Slash)
} else if options.get_flag(options::indicator_style::FILE_TYPE) {
IndicatorStyle::FileType
Some(IndicatorStyle::FileType)
} else {
IndicatorStyle::None
None
}
}

Expand Down Expand Up @@ -957,7 +951,7 @@ impl Config {
} else if options.get_flag(options::dereference::DIR_ARGS) {
Dereference::DirArgs
} else if options.get_flag(options::DIRECTORY)
|| indicator_style == IndicatorStyle::Classify
|| indicator_style == Some(IndicatorStyle::Classify)
|| format == Format::Long
{
Dereference::None
Expand Down
146 changes: 89 additions & 57 deletions src/uu/ls/src/display.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ use crate::colors::{StyleManager, color_name};
use crate::config::Files;
use crate::dired::{self, DiredOutput};
use crate::{Config, ListState, LsError, PathData, get_block_size};
use lscolors::Indicator;

/// Width of the standard Unix permissions string (one file-type char plus
/// nine permission bits, e.g. `drwxr-xr-x`).
Expand Down Expand Up @@ -91,9 +92,8 @@ pub(crate) struct DisplayItemName {
pub(crate) dired_name_len: usize,
}

#[derive(PartialEq, Eq)]
#[derive(Clone, Copy, PartialEq, Eq)]
pub(crate) enum IndicatorStyle {
None,
Slash,
FileType,
Classify,
Expand Down Expand Up @@ -705,35 +705,31 @@ fn display_item_name(
}
}

if config.indicator_style != IndicatorStyle::None {
let sym = classify_file(path);

let char_opt = match config.indicator_style {
IndicatorStyle::Classify => sym,
IndicatorStyle::FileType => {
// Don't append an asterisk.
match sym {
Some('*') => None,
_ => sym,
}
}
IndicatorStyle::Slash => {
// Append only a slash.
match sym {
Some('/') => Some('/'),
_ => None,
}
}
IndicatorStyle::None => None,
};
let is_long_symlink = config.format == Format::Long
&& path.file_type().is_some_and(FileType::is_symlink)
&& !path.must_dereference;

if let Some(c) = char_opt {
if !is_long_symlink {
if let Some(c) = indicator_char(path, config.indicator_style) {
let _ = name.write_char(c);
}
}

let dired_name_len = if config.dired { name.len() } else { 0 };

let has_mi_or_or = style_manager.as_ref().is_some_and(|sm| {
sm.has_indicator_style(Indicator::OrphanedSymbolicLink)
|| sm.has_indicator_style(Indicator::MissingFile)
});
// Only stat symlink target when:
// 1. Color is enabled AND LS_COLORS has mi= or or=, OR
// 2. Long format AND (--classify or --file-type)
let should_stat_target = has_mi_or_or
|| matches!(
config.indicator_style,
Some(IndicatorStyle::Classify) | Some(IndicatorStyle::FileType)
);

if config.format == Format::Long
&& path.file_type().is_some_and(FileType::is_symlink)
&& !path.must_dereference
Expand All @@ -745,10 +741,11 @@ fn display_item_name(
// We might as well color the symlink output after the arrow.
// This makes extra system calls, but provides important information that
// people run `ls -l --color` are very interested in.
if let Some(style_manager) = &mut style_manager {
let escaped_target = escape_name_with_locale(target_path.as_os_str(), config);
// We get the absolute path to be able to construct PathData with valid Metadata.
// This is because relative symlinks will fail to get_metadata.
let escaped_target = escape_name_with_locale(target_path.as_os_str(), config);

// We get the absolute path to be able to construct PathData with valid Metadata.
// This is because relative symlinks will fail to get_metadata.
if should_stat_target {
let absolute_target = if target_path.is_relative() {
match path.path().parent() {
Some(p) => &p.join(&target_path),
Expand All @@ -757,7 +754,6 @@ fn display_item_name(
} else {
&target_path
};

match fs::canonicalize(absolute_target) {
Ok(resolved_target) => {
let target_data = PathData::new(
Expand All @@ -769,43 +765,62 @@ fn display_item_name(
false,
);

// Check if the target actually needs coloring
Comment thread
joknarf marked this conversation as resolved.
let md_option: Option<Metadata> = target_data
.metadata()
.cloned()
.or_else(|| target_data.p_buf.symlink_metadata().ok());
let style = style_manager.colors.style_for_path_with_metadata(
&target_data.p_buf,
md_option.as_ref(),
);

if style.is_some() {
// Only apply coloring if there's actually a style
Comment thread
joknarf marked this conversation as resolved.
name.push(color_name(
escaped_target,
&target_data,
style_manager,
None,
is_wrap(name.len()),
));
let target_display = if let Some(style_manager) = style_manager {
let md = match target_data.metadata() {
Some(md) => Some(Cow::Borrowed(md)),
None => {
target_data.p_buf.symlink_metadata().ok().map(Cow::Owned)
}
};
// Check if the target actually needs coloring
if style_manager
.colors
.style_for_path_with_metadata(&target_data.p_buf, md.as_deref())
.is_some()
{
// Only apply coloring if there's actually a style
color_name(
escaped_target,
&target_data,
style_manager,
None,
is_wrap(name.len()),
)
} else {
// For regular files with no coloring, just use plain text
escaped_target
}
} else {
// For regular files with no coloring, just use plain text
Comment thread
joknarf marked this conversation as resolved.
name.push(escaped_target);
escaped_target
};
name.push(target_display);
// Add appropriate indicator based on indicator_style
if let Some(c) = indicator_char(&target_data, config.indicator_style) {
if matches!(
config.indicator_style,
Some(IndicatorStyle::Classify)
| Some(IndicatorStyle::FileType)
| Some(IndicatorStyle::Slash)
) {
let _ = name.write_char(c);
}
}
}
Err(_) => {
name.push(
style_manager.apply_missing_target_style(
if let Some(style_manager) = &mut style_manager {
name.push(style_manager.apply_missing_target_style(
escaped_target,
is_wrap(name.len()),
),
);
));
} else {
// If no coloring is required, we just use target as is.
// with the right quoting
name.push(escaped_target);
}
}
}
} else {
// If no coloring is required, we just use target as is.
// Apply the right quoting
name.push(escape_name_with_locale(target_path.as_os_str(), config));
name.push(&escaped_target);
}
}
Err(err) => {
Expand Down Expand Up @@ -1118,6 +1133,23 @@ fn display_item_long(
Ok(())
}

fn indicator_char(path: &PathData, style: Option<IndicatorStyle>) -> Option<char> {
let style = style?;
let sym = classify_file(path);
Comment thread
joknarf marked this conversation as resolved.

match style {
IndicatorStyle::Classify => sym,
IndicatorStyle::FileType => match sym {
Some('*') => None,
_ => sym,
},
IndicatorStyle::Slash => match sym {
Some('/') => Some('/'),
_ => None,
},
}
}

fn classify_file(path: &PathData) -> Option<char> {
let file_type = path.file_type()?;

Expand Down
Loading
Loading