Skip to content
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

Jump to bottom

ls --dired -R: fix the positions #5341

Merged
merged 10 commits into from
Oct 19, 2023
Merged

ls --dired -R: fix the positions #5341

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4fbd379
move get_offset_from_previous_line into a specific function
sylvestre Sep 30, 2023
05b2e4b
dired: improve the -R support
sylvestre Sep 30, 2023
7b825f1
dired: fix the display with subdir
sylvestre Sep 30, 2023
22aa74b
ls --dired -R: fix the positions
sylvestre Sep 30, 2023
7c034db
ls --dired -R: verify also the SUBDIRED coordinate
sylvestre Sep 30, 2023
68bcf23
ls --dired -R: add a long file name and fix a windows test
sylvestre Sep 30, 2023
289c31c
dired: always put dired first in the args + minor fixes
sylvestre Oct 4, 2023
9d6c75a
Merge branch 'main' into fix-subdir
sylvestre Oct 12, 2023
3395d9d
Merge branch 'main' into fix-subdir
sylvestre Oct 19, 2023
eab2fd2
ls: add cognitive_complexity to silent a warning
sylvestre Oct 19, 2023
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
204 changes: 176 additions & 28 deletions src/uu/ls/src/dired.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ use std::fmt;
use std::io::{BufWriter, Stdout, Write};
use uucore::error::UResult;

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq)]
pub struct BytePosition {
pub start: usize,
pub end: usize,
}

/// Represents the output structure for DIRED, containing positions for both DIRED and SUBDIRED.
#[derive(Debug, Clone, Default)]
#[derive(Debug, Clone, Default, PartialEq)]
pub struct DiredOutput {
pub dired_positions: Vec<BytePosition>,
pub subdired_positions: Vec<BytePosition>,
Expand All @@ -32,17 +32,21 @@ impl fmt::Display for BytePosition {
// When --dired is used, all lines starts with 2 spaces
static DIRED_TRAILING_OFFSET: usize = 2;

fn get_offset_from_previous_line(dired_positions: &[BytePosition]) -> usize {
if let Some(last_position) = dired_positions.last() {
last_position.end + 1
} else {
0
}
}

/// Calculates the byte positions for DIRED
pub fn calculate_dired(
dired_positions: &[BytePosition],
output_display_len: usize,
dfn_len: usize,
dired_positions: &[BytePosition],
) -> (usize, usize) {
let offset_from_previous_line = if let Some(last_position) = dired_positions.last() {
last_position.end + 1
} else {
0
};
let offset_from_previous_line = get_offset_from_previous_line(dired_positions);

let start = output_display_len + offset_from_previous_line;
let end = start + dfn_len;
Expand All @@ -55,15 +59,18 @@ pub fn indent(out: &mut BufWriter<Stdout>) -> UResult<()> {
}

pub fn calculate_subdired(dired: &mut DiredOutput, path_len: usize) {
let offset = if dired.subdired_positions.is_empty() {
DIRED_TRAILING_OFFSET
let offset_from_previous_line = get_offset_from_previous_line(&dired.dired_positions);

let additional_offset = if dired.subdired_positions.is_empty() {
0
} else {
dired.subdired_positions[dired.subdired_positions.len() - 1].start + DIRED_TRAILING_OFFSET
// if we have several directories: \n\n
2
};
dired.subdired_positions.push(BytePosition {
start: offset,
end: path_len + offset,
});

let start = offset_from_previous_line + DIRED_TRAILING_OFFSET + additional_offset;
let end = start + path_len;
dired.subdired_positions.push(BytePosition { start, end });
}

/// Prints the dired output based on the given configuration and dired structure.
Expand All @@ -73,10 +80,11 @@ pub fn print_dired_output(
out: &mut BufWriter<Stdout>,
) -> UResult<()> {
out.flush()?;
if dired.padding == 0 && !dired.dired_positions.is_empty() {
print_positions("//DIRED//", &dired.dired_positions);
}
if config.recursive {
print_positions("//SUBDIRED//", &dired.subdired_positions);
} else if dired.padding == 0 {
print_positions("//DIRED//", &dired.dired_positions);
}
println!("//DIRED-OPTIONS// --quoting-style={}", config.quoting_style);
Ok(())
Expand All @@ -91,17 +99,31 @@ fn print_positions(prefix: &str, positions: &Vec<BytePosition>) {
println!();
}

pub fn add_total(total_len: usize, dired: &mut DiredOutput) {
// when dealing with " total: xx", it isn't part of the //DIRED//
// so, we just keep the size line to add it to the position of the next file
dired.padding = total_len + DIRED_TRAILING_OFFSET;
pub fn add_total(dired: &mut DiredOutput, total_len: usize) {
if dired.padding == 0 {
sylvestre marked this conversation as resolved.
Show resolved Hide resolved
let offset_from_previous_line = get_offset_from_previous_line(&dired.dired_positions);
// when dealing with " total: xx", it isn't part of the //DIRED//
// so, we just keep the size line to add it to the position of the next file
dired.padding = total_len + offset_from_previous_line + DIRED_TRAILING_OFFSET;
} else {
// += because if we are in -R, we have " dir:\n total X". So, we need to take the
// previous padding too.
// and we already have the previous position in mind
dired.padding += total_len + DIRED_TRAILING_OFFSET;
}
}

// when using -R, we have the dirname. we need to add it to the padding
pub fn add_dir_name(dired: &mut DiredOutput, dir_len: usize) {
// 1 for the ":" in " dirname:"
dired.padding += dir_len + DIRED_TRAILING_OFFSET + 1;
}

/// Calculates byte positions and updates the dired structure.
pub fn calculate_and_update_positions(
dired: &mut DiredOutput,
output_display_len: usize,
dfn_len: usize,
dired: &mut DiredOutput,
) {
let offset = dired
.dired_positions
Expand All @@ -111,14 +133,14 @@ pub fn calculate_and_update_positions(
});
let start = output_display_len + offset + DIRED_TRAILING_OFFSET;
let end = start + dfn_len;
update_positions(start, end, dired);
update_positions(dired, start, end);
}

/// Updates the dired positions based on the given start and end positions.
/// update when it is the first element in the list (to manage "total X")
/// insert when it isn't the about total
pub fn update_positions(start: usize, end: usize, dired: &mut DiredOutput) {
// padding can be 0 but as it doesn't matter<
pub fn update_positions(dired: &mut DiredOutput, start: usize, end: usize) {
// padding can be 0 but as it doesn't matter
dired.dired_positions.push(BytePosition {
start: start + dired.padding,
end: end + dired.padding,
Expand All @@ -136,12 +158,112 @@ mod tests {
let output_display = "sample_output".to_string();
let dfn = "sample_file".to_string();
let dired_positions = vec![BytePosition { start: 5, end: 10 }];
let (start, end) = calculate_dired(output_display.len(), dfn.len(), &dired_positions);
let (start, end) = calculate_dired(&dired_positions, output_display.len(), dfn.len());

assert_eq!(start, 24);
assert_eq!(end, 35);
}

#[test]
fn test_get_offset_from_previous_line() {
let positions = vec![
BytePosition { start: 0, end: 3 },
BytePosition { start: 4, end: 7 },
BytePosition { start: 8, end: 11 },
];
assert_eq!(get_offset_from_previous_line(&positions), 12);
}
#[test]
fn test_calculate_subdired() {
let mut dired = DiredOutput {
dired_positions: vec![
BytePosition { start: 0, end: 3 },
BytePosition { start: 4, end: 7 },
BytePosition { start: 8, end: 11 },
],
subdired_positions: vec![],
padding: 0,
};
let path_len = 5;
calculate_subdired(&mut dired, path_len);
assert_eq!(
dired.subdired_positions,
vec![BytePosition { start: 14, end: 19 }],
);
}

#[test]
fn test_add_dir_name() {
let mut dired = DiredOutput {
dired_positions: vec![
BytePosition { start: 0, end: 3 },
BytePosition { start: 4, end: 7 },
BytePosition { start: 8, end: 11 },
],
subdired_positions: vec![],
padding: 0,
};
let dir_len = 5;
add_dir_name(&mut dired, dir_len);
assert_eq!(
dired,
DiredOutput {
dired_positions: vec![
BytePosition { start: 0, end: 3 },
BytePosition { start: 4, end: 7 },
BytePosition { start: 8, end: 11 },
],
subdired_positions: vec![],
// 8 = 1 for the \n + 5 for dir_len + 2 for " " + 1 for :
padding: 8
}
);
}

#[test]
fn test_add_total() {
let mut dired = DiredOutput {
dired_positions: vec![
BytePosition { start: 0, end: 3 },
BytePosition { start: 4, end: 7 },
BytePosition { start: 8, end: 11 },
],
subdired_positions: vec![],
padding: 0,
};
// if we have "total: 2"
let total_len = 8;
add_total(&mut dired, total_len);
// 22 = 8 (len) + 2 (padding) + 11 (previous position) + 1 (\n)
assert_eq!(dired.padding, 22);
}

#[test]
fn test_add_dir_name_and_total() {
// test when we have
// dirname:
// total 0
// -rw-r--r-- 1 sylvestre sylvestre 0 Sep 30 09:41 ab

let mut dired = DiredOutput {
dired_positions: vec![
BytePosition { start: 0, end: 3 },
BytePosition { start: 4, end: 7 },
BytePosition { start: 8, end: 11 },
],
subdired_positions: vec![],
padding: 0,
};
let dir_len = 5;
add_dir_name(&mut dired, dir_len);
// 8 = 2 (" ") + 1 (\n) + 5 + 1 (: of dirname)
assert_eq!(dired.padding, 8);

let total_len = 8;
add_total(&mut dired, total_len);
assert_eq!(dired.padding, 18);
}

#[test]
fn test_dired_update_positions() {
let mut dired = DiredOutput {
Expand All @@ -151,15 +273,41 @@ mod tests {
};

// Test with adjust = true
update_positions(15, 20, &mut dired);
update_positions(&mut dired, 15, 20);
let last_position = dired.dired_positions.last().unwrap();
assert_eq!(last_position.start, 25); // 15 + 10 (end of the previous position)
assert_eq!(last_position.end, 30); // 20 + 10 (end of the previous position)

// Test with adjust = false
update_positions(30, 35, &mut dired);
update_positions(&mut dired, 30, 35);
let last_position = dired.dired_positions.last().unwrap();
assert_eq!(last_position.start, 30);
assert_eq!(last_position.end, 35);
}

#[test]
fn test_calculate_and_update_positions() {
let mut dired = DiredOutput {
dired_positions: vec![
BytePosition { start: 0, end: 3 },
BytePosition { start: 4, end: 7 },
BytePosition { start: 8, end: 11 },
],
subdired_positions: vec![],
padding: 5,
};
let output_display_len = 15;
let dfn_len = 5;
calculate_and_update_positions(&mut dired, output_display_len, dfn_len);
assert_eq!(
dired.dired_positions,
vec![
BytePosition { start: 0, end: 3 },
BytePosition { start: 4, end: 7 },
BytePosition { start: 8, end: 11 },
BytePosition { start: 32, end: 37 },
]
);
assert_eq!(dired.padding, 0);
}
}
42 changes: 34 additions & 8 deletions src/uu/ls/src/ls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1856,7 +1856,10 @@ impl PathData {
}
}

#[allow(clippy::cognitive_complexity)]
fn show_dir_name(dir: &Path, out: &mut BufWriter<Stdout>) {
write!(out, "{}:", dir.display()).unwrap();
}

pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> {
let mut files = Vec::<PathData>::new();
let mut dirs = Vec::<PathData>::new();
Expand Down Expand Up @@ -1922,10 +1925,17 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> {
}
writeln!(out, "{}:", path_data.p_buf.display())?;
if config.dired {
dired::calculate_subdired(&mut dired, path_data.display_name.len());
// First directory displayed
let dir_len = path_data.display_name.len();
// add the //SUBDIRED// coordinates
dired::calculate_subdired(&mut dired, dir_len);
// Add the padding for the dir name
dired::add_dir_name(&mut dired, dir_len);
}
} else {
writeln!(out, "\n{}:", path_data.p_buf.display())?;
writeln!(out)?;
show_dir_name(&path_data.p_buf, &mut out);
writeln!(out)?;
}
}
let mut listed_ancestors = HashSet::new();
Expand Down Expand Up @@ -2104,7 +2114,7 @@ fn enter_directory(
let total = return_total(&entries, config, out)?;
write!(out, "{}", total.as_str())?;
if config.dired {
dired::add_total(total.len(), dired);
dired::add_total(dired, total.len());
}
}

Expand Down Expand Up @@ -2132,7 +2142,23 @@ fn enter_directory(
if listed_ancestors
.insert(FileInformation::from_path(&e.p_buf, e.must_dereference)?)
{
writeln!(out, "\n{}:", e.p_buf.display())?;
// when listing several directories in recursive mode, we show
// "dirname:" at the beginning of the file list
writeln!(out)?;
if config.dired {
// We already injected the first dir
// Continue with the others
// 2 = \n + \n
dired.padding = 2;
dired::indent(out)?;
let dir_name_size = e.p_buf.to_string_lossy().len();
dired::calculate_subdired(dired, dir_name_size);
// inject dir name
dired::add_dir_name(dired, dir_name_size);
}

show_dir_name(&e.p_buf, out);
writeln!(out)?;
enter_directory(e, rd, config, out, listed_ancestors, dired)?;
listed_ancestors
.remove(&FileInformation::from_path(&e.p_buf, e.must_dereference)?);
Expand Down Expand Up @@ -2547,11 +2573,11 @@ fn display_item_long(
let displayed_file = display_file_name(item, config, None, String::new(), out).contents;
if config.dired {
let (start, end) = dired::calculate_dired(
&dired.dired_positions,
output_display.len(),
displayed_file.len(),
&dired.dired_positions,
);
dired::update_positions(start, end, dired);
dired::update_positions(dired, start, end);
}
write!(output_display, "{}{}", displayed_file, config.line_ending).unwrap();
} else {
Expand Down Expand Up @@ -2639,9 +2665,9 @@ fn display_item_long(

if config.dired {
dired::calculate_and_update_positions(
dired,
output_display.len(),
displayed_file.trim().len(),
dired,
);
}
write!(output_display, "{}{}", displayed_file, config.line_ending).unwrap();
Expand Down
Loading
Loading