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

Open
joknarf wants to merge 3 commits into
base: main
Choose a base branch
from
+111 −86
Open

ls: ls -lF symlink target indicators #11554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c832ca8
ls: ls -lF symlink target indicator
joknarf Mar 30, 2026
e6fd64c
tests/ls: test for ls -lF symlink indicator
joknarf Mar 31, 2026
b5a1194
fix(ls): take care of performance regressions
joknarf Mar 31, 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 @@ -172,7 +172,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 @@ -554,34 +554,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 @@ -954,7 +948,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
138 changes: 73 additions & 65 deletions src/uu/ls/src/display.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,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 @@ -704,29 +703,12 @@ 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);
}
}
Expand All @@ -744,66 +726,75 @@ 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 absolute_target = if target_path.is_relative() {
match path.path().parent() {
Some(p) => &p.join(&target_path),
None => &target_path,
}
} else {
&target_path
};

match fs::canonicalize(absolute_target) {
Ok(resolved_target) => {
let target_data = PathData::new(
resolved_target.as_path().into(),
None,
target_path.file_name().map(Cow::Borrowed),
config,
false,
);

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 absolute_target = if target_path.is_relative() {
match path.path().parent() {
Some(p) => &p.join(&target_path),
None => &target_path,
}
} else {
&target_path
};

match fs::canonicalize(absolute_target) {
Ok(resolved_target) => {
let target_data = PathData::new(
resolved_target.as_path().into(),
None,
target_path.file_name().map(Cow::Borrowed),
config,
false,
);

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
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() {
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
name.push(color_name(
&color_name(
escaped_target,
&target_data,
style_manager,
None,
is_wrap(name.len()),
));
)
} else {
// For regular files with no coloring, just use plain text
name.push(escaped_target);
&escaped_target
}
} else {
&escaped_target
};

name.push(target_display);
if let Some(c) = indicator_char(&target_data, config.indicator_style) {
let _ = name.write_char(c);
}
Err(_) => {
}
Err(_) => {
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));
}
}
Err(err) => {
Expand Down Expand Up @@ -1118,6 +1109,23 @@ fn display_item_long(
Ok(())
}

fn indicator_char(path: &PathData, style: Option<IndicatorStyle>) -> Option<char> {
let style = style?;
let sym = classify_file(path);

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
23 changes: 23 additions & 0 deletions tests/by-util/test_ls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3386,6 +3386,29 @@ fn test_ls_indicator_style() {
}
}

#[test]
#[cfg(not(windows))]
fn test_ls_indicator_style_symlink_target_long() {
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;

at.mkdir("dir");
assert!(at.dir_exists("dir"));

at.symlink_dir("dir", "dir_link");
assert!(at.is_symlink("dir_link"));

scene
.ucmd()
.arg("--classify")
.arg("-l")
.arg("dir_link")
.succeeds()
.stdout_contains("dir_link -> ")
.stdout_does_not_contain("dir_link@ -> ")
.stdout_contains("/dir/");
}

// Essentially the same test as above, but only test symlinks and directories,
// not pipes or sockets.
#[test]
Expand Down
Loading