Skip to content

Commit 6ac3af0

Browse files
authored
feat(builtins): add ls -C multi-column output (#1079)
## Summary - Adds `-C` flag to `ls` builtin for multi-column output (POSIX standard) - Column-major ordering matching GNU coreutils behavior - `-F` was already supported; `-C` was the only missing flag from the issue - Adds spec test for `ls -C` and `ls -CF` Closes #1069
1 parent 0365f14 commit 6ac3af0

File tree

2 files changed

+124
-10
lines changed

2 files changed

+124
-10
lines changed

crates/bashkit/src/builtins/ls.rs

Lines changed: 95 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,12 @@ struct LsOptions {
2020
recursive: bool,
2121
sort_by_time: bool,
2222
classify: bool,
23+
columns: bool,
2324
}
2425

2526
/// The ls builtin - list directory contents.
2627
///
27-
/// Usage: ls [-l] [-a] [-h] [-1] [-R] [-t] [-F] [PATH...]
28+
/// Usage: ls [-l] [-a] [-h] [-1] [-R] [-t] [-F] [-C] [PATH...]
2829
///
2930
/// Options:
3031
/// -l Use long listing format
@@ -34,6 +35,7 @@ struct LsOptions {
3435
/// -R List subdirectories recursively
3536
/// -t Sort by modification time, newest first
3637
/// -F Append indicator (/ for dirs, * for executables, @ for symlinks, | for FIFOs)
38+
/// -C List entries in columns (multi-column output)
3739
pub struct Ls;
3840

3941
#[async_trait]
@@ -47,6 +49,7 @@ impl Builtin for Ls {
4749
recursive: false,
4850
sort_by_time: false,
4951
classify: false,
52+
columns: false,
5053
};
5154

5255
// Parse flags
@@ -64,6 +67,7 @@ impl Builtin for Ls {
6467
'R' => opts.recursive = true,
6568
't' => opts.sort_by_time = true,
6669
'F' => opts.classify = true,
70+
'C' => opts.columns = true,
6771
_ => {
6872
return Ok(ExecResult::err(
6973
format!("ls: invalid option -- '{}'\n", c),
@@ -118,8 +122,8 @@ impl Builtin for Ls {
118122
}
119123

120124
// Output file arguments first (preserving path as given by user)
121-
for (path_str, metadata) in &file_args {
122-
if opts.long {
125+
if opts.long {
126+
for (path_str, metadata) in &file_args {
123127
let mut entry = format_long_entry(path_str, metadata, opts.human);
124128
if opts.classify {
125129
// Insert suffix before the trailing newline
@@ -129,12 +133,25 @@ impl Builtin for Ls {
129133
}
130134
}
131135
output.push_str(&entry);
136+
}
137+
} else if !file_args.is_empty() {
138+
let names: Vec<String> = file_args
139+
.iter()
140+
.map(|(path_str, metadata)| {
141+
let mut name = (*path_str).to_string();
142+
if opts.classify {
143+
name.push_str(classify_suffix(metadata));
144+
}
145+
name
146+
})
147+
.collect();
148+
if opts.columns && !opts.one_per_line {
149+
output.push_str(&format_columns(&names, 80));
132150
} else {
133-
output.push_str(path_str);
134-
if opts.classify {
135-
output.push_str(classify_suffix(metadata));
151+
for name in &names {
152+
output.push_str(name);
153+
output.push('\n');
136154
}
137-
output.push('\n');
138155
}
139156
}
140157

@@ -219,19 +236,31 @@ async fn list_directory(
219236
}
220237
}
221238
} else {
239+
// Collect entry names for potential column formatting
240+
let mut names: Vec<String> = Vec::new();
222241
for entry in &filtered {
223-
output.push_str(&entry.name);
242+
let mut name = entry.name.clone();
224243
if opts.classify {
225-
output.push_str(classify_suffix(&entry.metadata));
244+
name.push_str(classify_suffix(&entry.metadata));
226245
}
227-
output.push('\n');
246+
names.push(name);
228247
if opts.recursive && entry.metadata.file_type.is_dir() {
229248
subdirs.push((
230249
path.join(&entry.name),
231250
format!("{}/{}", display_path, entry.name),
232251
));
233252
}
234253
}
254+
255+
// Precedence: -l > -1 > -C > default (one-per-line)
256+
if opts.columns && !opts.one_per_line {
257+
output.push_str(&format_columns(&names, 80));
258+
} else {
259+
for name in &names {
260+
output.push_str(name);
261+
output.push('\n');
262+
}
263+
}
235264
}
236265

237266
// Recursive listing
@@ -262,6 +291,62 @@ fn classify_suffix(metadata: &crate::fs::Metadata) -> &'static str {
262291
}
263292
}
264293

294+
/// Format entries in column-major order, like `ls -C`.
295+
/// Uses a fixed terminal width (80) since VFS has no real terminal.
296+
/// Per-column widths match GNU coreutils behavior.
297+
fn format_columns(entries: &[String], terminal_width: usize) -> String {
298+
if entries.is_empty() {
299+
return String::new();
300+
}
301+
302+
// Try fitting as many columns as possible, starting from the maximum
303+
let max_width = entries.iter().map(|e| e.len()).max().unwrap_or(0);
304+
let max_possible_cols = (terminal_width / (max_width.min(1) + 2)).max(1);
305+
306+
let mut num_cols = 1;
307+
let mut col_widths: Vec<usize> = vec![0];
308+
let mut num_rows = entries.len();
309+
310+
// Try increasing column counts to find the best fit
311+
for try_cols in 2..=max_possible_cols.min(entries.len()) {
312+
let try_rows = entries.len().div_ceil(try_cols);
313+
// Calculate per-column widths (max entry width in each column)
314+
let mut widths = vec![0usize; try_cols];
315+
for (i, entry) in entries.iter().enumerate() {
316+
let col = i / try_rows;
317+
if col < try_cols {
318+
widths[col] = widths[col].max(entry.len());
319+
}
320+
}
321+
// Total width: each column except last gets 2-space padding
322+
let total: usize = widths.iter().sum::<usize>() + (try_cols - 1) * 2;
323+
if total <= terminal_width {
324+
num_cols = try_cols;
325+
col_widths = widths;
326+
num_rows = try_rows;
327+
}
328+
}
329+
330+
let mut output = String::new();
331+
for row in 0..num_rows {
332+
for (col, col_w) in col_widths.iter().enumerate() {
333+
// Column-major order: fill columns top-to-bottom, left-to-right
334+
let idx = col * num_rows + row;
335+
if idx < entries.len() {
336+
let is_last = col == num_cols - 1 || idx + num_rows >= entries.len();
337+
if is_last {
338+
output.push_str(&entries[idx]);
339+
} else {
340+
let width = col_w + 2; // entry width + 2 spaces
341+
output.push_str(&format!("{:<width$}", entries[idx], width = width));
342+
}
343+
}
344+
}
345+
output.push('\n');
346+
}
347+
output
348+
}
349+
265350
fn format_long_entry(name: &str, metadata: &crate::fs::Metadata, human: bool) -> String {
266351
let file_type = match metadata.file_type {
267352
FileType::Directory => 'd',

crates/bashkit/tests/spec_cases/bash/ls.test.sh

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,35 @@ ls -F /tmp/lscf/mydir /tmp/lscf/normal.txt
7575
/tmp/lscf/mydir:
7676
### end
7777

78+
### ls_columns_basic
79+
# ls -C should produce multi-column output
80+
mkdir -p /tmp/lscol
81+
touch /tmp/lscol/alpha /tmp/lscol/beta /tmp/lscol/delta /tmp/lscol/gamma
82+
ls -C /tmp/lscol
83+
### expect
84+
alpha beta delta gamma
85+
### end
86+
87+
### ls_columns_with_classify
88+
# ls -CF should combine classify and columns
89+
mkdir -p /tmp/lscf2/subdir
90+
touch /tmp/lscf2/file.txt
91+
ls -CF /tmp/lscf2
92+
### expect
93+
file.txt subdir/
94+
### end
95+
96+
### ls_one_per_line_overrides_columns
97+
# ls -1 should override -C (one per line)
98+
mkdir -p /tmp/ls1c
99+
touch /tmp/ls1c/aaa /tmp/ls1c/bbb /tmp/ls1c/ccc
100+
ls -C1 /tmp/ls1c
101+
### expect
102+
aaa
103+
bbb
104+
ccc
105+
### end
106+
78107
### ls_classify_long
79108
### bash_diff: bashkit ls -l omits 'total' line
80109
# ls -lF should append indicators in long format

0 commit comments

Comments
 (0)