ls: track permissions column width independently

Replace the `has_alt_access` boolean (which widened the link-count column
to reserve space for the ACL/context indicator) with a `permissions`
field on `PaddingCollection`, so each column owns its own width.

Also make ACL detection in the padding-calculation pass use `item.path()`
to match the render path (previously used `item.display_name()`).

Refs #11790
This commit is contained in:
Sylvestre Ledru
2026-05-24 16:21:20 +02:00
parent 223fcaef79
commit c8af53a77f
+46 -32
View File
@@ -4,7 +4,7 @@
// file that was distributed with this source code.
// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm stringly
// spell-checker:ignore nohash strtime clocale ilog
// spell-checker:ignore nohash strtime clocale ilog drwxr
use core::ops::RangeInclusive;
use std::cell::LazyCell;
@@ -53,6 +53,10 @@ use crate::config::Files;
use crate::dired::{self, DiredOutput};
use crate::{Config, ListState, LsError, PathData, get_block_size};
/// Width of the standard Unix permissions string (one file-type char plus
/// nine permission bits, e.g. `drwxr-xr-x`).
const PERMISSIONS_WIDTH: usize = 10;
// Fields that can be removed or added to the long format
pub(crate) struct LongFormat {
pub(crate) author: bool,
@@ -75,10 +79,11 @@ pub(crate) struct PaddingCollection {
#[cfg(unix)]
pub(crate) minor: usize,
pub(crate) block_size: usize,
/// True if any listed item has an ACL or non-trivial security context,
/// which requires reserving one extra column for the `+`/`.` indicator
/// so link-count columns align across items with and without it.
pub(crate) has_alt_access: bool,
/// Width of the permissions field. [`PERMISSIONS_WIDTH`] by default
/// (e.g. `drwxr-xr-x`), or one more when any listed item has an ACL or
/// non-trivial security context and an extra column must be reserved for
/// the `+`/`.` indicator.
pub(crate) permissions: usize,
}
pub(crate) struct DisplayItemName {
@@ -302,6 +307,20 @@ fn pad_left(string: &str, count: usize) -> String {
format!("{string:>count$}")
}
/// Returns the alternate-access indicator that follows the 9-bit permission
/// string in long-format output: `.` for a non-trivial security context, `+`
/// for an ACL, otherwise a space (which acts as a placeholder so columns line
/// up across items that don't carry an indicator themselves).
fn alt_access_indicator(item: &PathData, config: &Config, is_acl_set: bool) -> u8 {
if item.security_context(config).len() > 1 {
b'.'
} else if is_acl_set {
b'+'
} else {
b' '
}
}
#[allow(clippy::cognitive_complexity)]
pub fn display_items(
items: &[PathData],
@@ -878,20 +897,15 @@ fn display_item_long(
state
.display_buf
.extend(display_permissions(md, true).as_bytes());
if item.security_context(config).len() > 1 {
// GNU `ls` uses a "." character to indicate a file with a security context,
// but not other alternate access method.
state.display_buf.push(b'.');
} else if is_acl_set {
state.display_buf.push(b'+');
} else {
state.display_buf.push(b' ');
if padding.permissions > PERMISSIONS_WIDTH {
state
.display_buf
.push(alt_access_indicator(item, config, is_acl_set));
}
state.display_buf.extend_pad_left(
&display_symlink_count(md),
padding.link_count + usize::from(padding.has_alt_access),
);
state.display_buf.push(b' ');
state
.display_buf
.extend_pad_left(&display_symlink_count(md), padding.link_count);
if config.long.owner {
state.display_buf.push(b' ');
@@ -1036,16 +1050,15 @@ fn display_item_long(
state.display_buf.push(leading_char as u8);
state.display_buf.extend(b"?????????");
if item.security_context(config).len() > 1 {
// GNU `ls` uses a "." character to indicate a file with a security context,
// but not other alternate access method.
state.display_buf.push(b'.');
if padding.permissions > PERMISSIONS_WIDTH {
// Metadata is unknown, so we can't probe for ACLs; only the
// security-context indicator is detectable here.
state
.display_buf
.push(alt_access_indicator(item, config, false));
}
state.display_buf.push(b' ');
state.display_buf.extend_pad_left(
"?",
padding.link_count + usize::from(padding.has_alt_access),
);
state.display_buf.extend_pad_left("?", padding.link_count);
if config.long.owner {
state.display_buf.push(b' ');
@@ -1228,7 +1241,7 @@ fn calculate_padding_collection(
major: 1,
minor: 1,
block_size: 1,
has_alt_access: false,
permissions: PERMISSIONS_WIDTH,
};
for item in items {
@@ -1260,16 +1273,17 @@ fn calculate_padding_collection(
padding_collections.context = context_len.max(padding_collections.context);
}
// Track whether any item has an ACL or non-trivial security context so
// rendering can reserve a single extra column for the `+`/`.` indicator.
// If any item has an ACL or non-trivial security context, widen
// the permissions column by one to reserve space for the `+`/`.`
// indicator.
{
#[cfg(any(not(unix), target_os = "android", target_os = "macos"))]
// TODO: See how Mac should work here
let is_acl_set = false;
#[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
let is_acl_set = has_acl(item.display_name());
let is_acl_set = has_acl(item.path());
if context_len > 1 || is_acl_set {
padding_collections.has_alt_access = true;
padding_collections.permissions = PERMISSIONS_WIDTH + 1;
}
}
@@ -1310,7 +1324,7 @@ fn calculate_padding_collection(
context: 1,
size: 1,
block_size: 1,
has_alt_access: false,
permissions: PERMISSIONS_WIDTH,
};
for item in items {