mirror of
https://github.com/nushell/nushell.git
synced 2024-12-23 07:30:13 +01:00
Skip decoration lines for detect columns --guess
(#13274)
# Description I introduced a regression in #13272 that resulted in `detect columns --guess` to panic whenever it had to handle empty, whitespace-only, or non-whitespace-only lines that go all the way to the last column (and as such, cannot be considered to be lines that only have entries for the first colum). I fix this by detecting these cases and skipping them, since these are usually decoration lines. An example is the second line output by `winget list`: ![image](https://github.com/nushell/nushell/assets/20356389/06c873fb-0a26-45dd-b020-3bcc737d027f) What we don't want to skip, however, is lines that contain no whitespace, and fit into the detected first column, since these lines represent cases where data is only available for the first column, and are not just decoration lines. For example (made up example, there are no such entries in `winget lits`'s output), in this output we would not want to skip the `Docker Desktop` line : ``` Name Id Version Available Source ------------------------------------------------------------------------------------------------------------------------------------- AMD Software ARPMachineX64AMD Catalyst Install Manager 24.4.1 AMD Ryzen Master ARPMachineX64AMD Ryzen Master 2.13.0.2908 Docker Desktop Mozilla Firefox (x64 en-US) Mozilla.Firefox 127.0.2 winget ``` ![image](https://github.com/nushell/nushell/assets/20356389/12e31995-a7c1-4759-8c62-fb4fb199fd2e) NOTE: `winget list | detect columns --guess` does not panic, but sadly still does not work as expected. I believe this is not a nushell issue anymore, but a `winget` one. When being piped, `winget` seems to add extra whitespace and random `\r` symbols at the beginning of the text. This messes with the column detection, of course. ![image](https://github.com/nushell/nushell/assets/20356389/7d1b7e5f-17d0-41c8-8d2f-7896e0d73d66) ![image](https://github.com/nushell/nushell/assets/20356389/56917954-1231-43e7-bacf-e5760e263054) ![image](https://github.com/nushell/nushell/assets/20356389/630bcfc9-eb78-4a45-9c8f-97efc0c224f4) # User-Facing Changes `detect columns --guess` should not panic when receiving output from `winget list` at all anymore. A breaking change is the skipping of decoration lines, especially since scripts probably were doing something like `winget list | lines | reject 1 | str join "\n" | detect columns --guess`. This will now cause them to reject a line with valid data. # Tests + Formatting Added tests that exercise these edge cases, as well as a single-column test to make sure that trivial cases keep working. # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. -->
This commit is contained in:
parent
a2873336bb
commit
69e4790b00
@ -72,7 +72,9 @@ impl GuessWidth {
|
||||
|
||||
let mut rows = Vec::new();
|
||||
while let Ok(columns) = self.read() {
|
||||
rows.push(columns);
|
||||
if !columns.is_empty() {
|
||||
rows.push(columns);
|
||||
}
|
||||
}
|
||||
rows
|
||||
}
|
||||
@ -180,6 +182,19 @@ fn split(line: &str, pos: &[usize], trim_space: bool) -> Vec<String> {
|
||||
let (line_char_boundaries, line_chars): (Vec<usize>, Vec<char>) = line.char_indices().unzip();
|
||||
let mut w = 0;
|
||||
|
||||
if line_chars.is_empty() || line_chars.iter().all(|&c| c.is_whitespace()) {
|
||||
// current line is completely empty, or only filled with whitespace
|
||||
return Vec::new();
|
||||
} else if !pos.is_empty()
|
||||
&& line_chars.iter().all(|&c| !c.is_whitespace())
|
||||
&& pos[0] < UnicodeWidthStr::width(line)
|
||||
{
|
||||
// we have more than 1 column in the input, but the current line has no whitespace,
|
||||
// and it is longer than the first detected column separation position
|
||||
// this indicates some kind of decoration line. let's skip it
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
for p in 0..line_char_boundaries.len() {
|
||||
if pos.is_empty() || n > pos.len() - 1 {
|
||||
start_char = p;
|
||||
@ -463,6 +478,122 @@ Ștefan Țincu ";
|
||||
assert_eq!(got, want);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_guess_width_single_column() {
|
||||
let input = "A
|
||||
|
||||
B
|
||||
|
||||
C";
|
||||
|
||||
let r = Box::new(std::io::BufReader::new(input.as_bytes())) as Box<dyn std::io::Read>;
|
||||
let reader = std::io::BufReader::new(r);
|
||||
|
||||
let mut guess_width = GuessWidth {
|
||||
reader,
|
||||
pos: Vec::new(),
|
||||
pre_lines: Vec::new(),
|
||||
pre_count: 0,
|
||||
limit_split: 0,
|
||||
};
|
||||
|
||||
let want = vec![vec!["A"], vec!["B"], vec!["C"]];
|
||||
let got = guess_width.read_all();
|
||||
assert_eq!(got, want);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_guess_width_row_without_whitespace() {
|
||||
let input = "A B C D
|
||||
-------
|
||||
E F G H";
|
||||
|
||||
let r = Box::new(std::io::BufReader::new(input.as_bytes())) as Box<dyn std::io::Read>;
|
||||
let reader = std::io::BufReader::new(r);
|
||||
|
||||
let mut guess_width = GuessWidth {
|
||||
reader,
|
||||
pos: Vec::new(),
|
||||
pre_lines: Vec::new(),
|
||||
pre_count: 0,
|
||||
limit_split: 0,
|
||||
};
|
||||
|
||||
let want = vec![vec!["A", "B", "C", "D"], vec!["E", "F", "G", "H"]];
|
||||
let got = guess_width.read_all();
|
||||
assert_eq!(got, want);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_guess_width_row_with_single_column() {
|
||||
let input = "A B C D
|
||||
E
|
||||
F G H I";
|
||||
|
||||
let r = Box::new(std::io::BufReader::new(input.as_bytes())) as Box<dyn std::io::Read>;
|
||||
let reader = std::io::BufReader::new(r);
|
||||
|
||||
let mut guess_width = GuessWidth {
|
||||
reader,
|
||||
pos: Vec::new(),
|
||||
pre_lines: Vec::new(),
|
||||
pre_count: 0,
|
||||
limit_split: 0,
|
||||
};
|
||||
|
||||
let want = vec![
|
||||
vec!["A", "B", "C", "D"],
|
||||
vec!["E"],
|
||||
vec!["F", "G", "H", "I"],
|
||||
];
|
||||
let got = guess_width.read_all();
|
||||
assert_eq!(got, want);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_guess_width_empty_row() {
|
||||
let input = "A B C D
|
||||
|
||||
E F G H";
|
||||
|
||||
let r = Box::new(std::io::BufReader::new(input.as_bytes())) as Box<dyn std::io::Read>;
|
||||
let reader = std::io::BufReader::new(r);
|
||||
|
||||
let mut guess_width = GuessWidth {
|
||||
reader,
|
||||
pos: Vec::new(),
|
||||
pre_lines: Vec::new(),
|
||||
pre_count: 0,
|
||||
limit_split: 0,
|
||||
};
|
||||
|
||||
let want = vec![vec!["A", "B", "C", "D"], vec!["E", "F", "G", "H"]];
|
||||
let got = guess_width.read_all();
|
||||
assert_eq!(got, want);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_guess_width_row_with_only_whitespace() {
|
||||
let input = "A B C D
|
||||
|
||||
E F G H";
|
||||
|
||||
let r = Box::new(std::io::BufReader::new(input.as_bytes())) as Box<dyn std::io::Read>;
|
||||
let reader = std::io::BufReader::new(r);
|
||||
|
||||
let mut guess_width = GuessWidth {
|
||||
reader,
|
||||
pos: Vec::new(),
|
||||
pre_lines: Vec::new(),
|
||||
pre_count: 0,
|
||||
limit_split: 0,
|
||||
};
|
||||
|
||||
let want = vec![vec!["A", "B", "C", "D"], vec!["E", "F", "G", "H"]];
|
||||
let got = guess_width.read_all();
|
||||
assert_eq!(got, want);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_to_table() {
|
||||
let lines = vec![
|
||||
|
Loading…
Reference in New Issue
Block a user