Skip to content

add \t filling option #46

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.

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

Merged
merged 24 commits into from
Apr 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ jobs:
grcov . --binary-path="${COVERAGE_REPORT_DIR}" --output-type lcov --output-path "${COVERAGE_REPORT_FILE}" --branch --ignore build.rs --ignore "vendor/*" --ignore "/*" --ignore "[a-zA-Z]:/*" --excl-br-line "^\s*((debug_)?assert(_eq|_ne)?!|#\[derive\()"
echo "name=report::${COVERAGE_REPORT_FILE}" >> $GITHUB_OUTPUT
- name: Upload coverage results (to Codecov.io)
uses: codecov/codecov-action@v4
uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
file: ${{ steps.coverage.outputs.report }}
Expand Down
9 changes: 7 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ and a set of options.
There are three options that must be specified in the [`GridOptions`] value that
dictate how the grid is formatted:

- [`filling`][filling]: what to put in between two columns — either a number of
spaces, or a text string;
- [`filling`][filling]: how to fill empty space between columns:
- [`Filling::Spaces`][Spaces] number of spaces between columns;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make the tabs option a part of the spaces option maybe? What do you think? Something like

enum Filling {
    Whitespace {
        spaces: usize,
        tab_size: Option<usize>,
    }
    Text(/* ... */),
}

What you got works pretty well too, but maybe it would be nice to also configure the separator size for a tab layout. I'd accept this too though!

Copy link
Contributor Author

@emar-kar emar-kar Apr 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying not to break the API, but introduce a new thing. Maybe we can add separator size in new Tab instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. I like your suggestion!

- [`Filling::Text`][Text] text string separator between columns;
- [`Filling::Tabs`][Tabs] special option which allows to set number of spaces between columns and set the size of `\t` character.
- [`direction`][direction]: specifies whether the cells should go along rows, or
columns:
- [`Direction::LeftToRight`][LeftToRight] starts them in the top left and
Expand Down Expand Up @@ -94,6 +96,9 @@ nine ten eleven twelve
[width]: https://docs.rs/uutils_term_grid/latest/term_grid/struct.GridOptions.html#structfield.width
[LeftToRight]: https://docs.rs/uutils_term_grid/latest/term_grid/enum.Direction.html#variant.LeftToRight
[TopToBottom]: https://docs.rs/uutils_term_grid/latest/term_grid/enum.Direction.html#variant.TopToBottom
[Spaces]: https://docs.rs/uutils_term_grid/latest/term_grid/enum.Filling.html#variant.Spaces
[Text]: https://docs.rs/uutils_term_grid/latest/term_grid/enum.Filling.html#variant.Text
[Tabs]:https://docs.rs/uutils_term_grid/latest/term_grid/enum.Filling.html#variant.Tabs

## Width of grid cells

Expand Down
2 changes: 1 addition & 1 deletion examples/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use term_grid::{Direction, Filling, Grid, GridOptions};
// 8 | 1024 | 131072 | 16777216 | 2147483648 | 274877906944 | 35184372088832
// 16 | 2048 | 262144 | 33554432 | 4294967296 | 549755813888 | 70368744177664
// 32 | 4096 | 524288 | 67108864 | 8589934592 | 1099511627776 | 140737488355328
// 64 | 8192 | 1048576 | 134217728 | 17179869184 | 2199023255552 |
// 64 | 8192 | 1048576 | 134217728 | 17179869184 | 2199023255552

fn main() {
let cells: Vec<_> = (0..48).map(|i| 2_isize.pow(i).to_string()).collect();
Expand Down
32 changes: 32 additions & 0 deletions examples/tabs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.

use term_grid::{Direction, Filling, Grid, GridOptions, DEFAULT_SEPARATOR_SIZE};

// This produces:
//
// 1···128↹··16384↹···2097152····268435456↹···34359738368├┤··4398046511104␊
// 2···256↹··32768↹···4194304····536870912↹···68719476736├┤··8796093022208␊
// 4···512↹··65536↹···8388608····1073741824···137438953472↹··17592186044416␊
// 8···1024··131072···16777216···2147483648···274877906944↹··35184372088832␊
// 16··2048··262144···33554432···4294967296···549755813888↹··70368744177664␊
// 32··4096··524288···67108864···8589934592···1099511627776··140737488355328␊
// 64··8192··1048576··134217728··17179869184··2199023255552␊

fn main() {
let cells: Vec<_> = (0..48).map(|i| 2_isize.pow(i).to_string()).collect();

let grid = Grid::new(
cells,
GridOptions {
direction: Direction::TopToBottom,
filling: Filling::Tabs {
spaces: DEFAULT_SEPARATOR_SIZE,
tab_size: 8,
},
width: 80,
},
);

println!("{}", grid);
}
93 changes: 76 additions & 17 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
use ansi_width::ansi_width;
use std::fmt;

/// Number of spaces in one \t.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor improvement is to put the character between backticks to make it nicer in rustdoc: `\t`

This only goes for the doc comments of course.

pub const SPACES_IN_TAB: usize = 8;

/// Default size for separator in spaces.
pub const DEFAULT_SEPARATOR_SIZE: usize = 2;

/// Direction cells should be written in: either across or downwards.
#[derive(PartialEq, Eq, Debug, Copy, Clone)]
pub enum Direction {
Expand All @@ -37,13 +43,22 @@ pub enum Filling {
///
/// `"|"` is a common choice.
Text(String),

/// Fill spaces with `\t`
Tabs {
/// A number of spaces
spaces: usize,
/// Size of `\t` in spaces
tab_size: usize,
},
}

impl Filling {
fn width(&self) -> usize {
match self {
Filling::Spaces(w) => *w,
Filling::Text(t) => ansi_width(t),
Filling::Tabs { spaces, .. } => *spaces,
}
}
}
Expand Down Expand Up @@ -257,9 +272,15 @@ impl<T: AsRef<str>> Grid<T> {

impl<T: AsRef<str>> fmt::Display for Grid<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
let separator = match &self.options.filling {
Filling::Spaces(n) => " ".repeat(*n),
Filling::Text(s) => s.clone(),
// If cells are empty then, nothing to print, skip.
if self.cells.is_empty() {
return Ok(());
}

let (tab_size, separator) = match &self.options.filling {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether using Option<usize> (or maybe Option<NonZeroUsize>) might be nicer here, because 0 has a special meaning here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I got you right, you want to do something like:

let (tab_size, separator) = match &self.options.filling {
    Filling::Spaces(n) => (None, " ".repeat(*n)),
    Filling::Text(s) => (None, s.clone()),
    Filling::Tabs { spaces, tab_size } => (Some(*tab_size), " ".repeat(*spaces)),
};

// ...

match tab_size {
    Some(tab_size) => {}, // Calculate tabs.
    None => {}, // Print spaces.
}

If that's what you mean, I'll disagree in this design. With this change I can do something stupid as pass Some(0) as a tab_size and instead of simply filling everything with spaces, it will still try to calculate the tabs size.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is indeed what I would suggest, but the 0 tab size indeed should not be possible, so it would be:

Filling::Tabs { spaces, tabsize } => (NonZeroUsize::new(*tab_size), " ".repeat(*spaces)),

It would functionally be very similar to what you have, but it would make it very clear that None is a very different case than positive numbers. Feel free to leave it like this though. It's probably clear enough.

See https://doc.rust-lang.org/stable/std/num/type.NonZeroUsize.html for more info on that type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the link, didn't think about that before. I tried this solution, but got an error in calculations of the tabs cannot subtract usize from NonZero<usize>. Even tho I like the suggestion, but is it reasonable to add more types conversions to handle this update?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! You can just do a get after matching on the option to get a usize

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've read about it. But my question is mostly how justified is it, to add new type, just to replace if-else on this:

match tab_size {
    Some(tab_size) => {
        let tab_size = tab_size.get();
        // ...
        }
    None => {
        // ...
    }
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could write that with an if let if you prefer:

if let Some(tab_size) = tab_size {

} else {

}

But I do see your point. It's up to you!

Filling::Spaces(n) => (0, " ".repeat(*n)),
Filling::Text(s) => (0, s.clone()),
Filling::Tabs { spaces, tab_size } => (*tab_size, " ".repeat(*spaces)),
};

// Initialize a buffer of spaces. The idea here is that any cell
Expand All @@ -270,24 +291,33 @@ impl<T: AsRef<str>> fmt::Display for Grid<T> {
// We overestimate how many spaces we need, but this is not
// part of the loop and it's therefore not super important to
// get exactly right.
let padding = " ".repeat(self.widest_cell_width);
let padding = " ".repeat(self.widest_cell_width + self.options.filling.width());

for y in 0..self.dimensions.num_lines {
// Current position on the line.
let mut cursor: usize = 0;
for x in 0..self.dimensions.widths.len() {
let num = match self.options.direction {
Direction::LeftToRight => y * self.dimensions.widths.len() + x,
Direction::TopToBottom => y + self.dimensions.num_lines * x,
// Calculate position of the current element of the grid
// in cells and widths vectors and the offset to the next value.
let (current, offset) = match self.options.direction {
Direction::LeftToRight => (y * self.dimensions.widths.len() + x, 1),
Direction::TopToBottom => {
(y + self.dimensions.num_lines * x, self.dimensions.num_lines)
}
};

// Abandon a line mid-way through if that’s where the cells end
if num >= self.cells.len() {
continue;
// Abandon a line mid-way through if that’s where the cells end.
if current >= self.cells.len() {
break;
}

let contents = &self.cells[num];
let width = self.widths[num];
// Last in row checks only the predefined grid width.
// It does not check if there will be more entries.
// For this purpose we define next value as well.
// This prevents printing separator after the actual last value in a row.
let last_in_row = x == self.dimensions.widths.len() - 1;

let contents = &self.cells[current];
let width = self.widths[current];
let col_width = self.dimensions.widths[x];
let padding_size = col_width - width;

Expand All @@ -305,11 +335,40 @@ impl<T: AsRef<str>> fmt::Display for Grid<T> {
// We also only call `write_str` when we actually need padding as
// another optimization.
f.write_str(contents.as_ref())?;
if !last_in_row {
if padding_size > 0 {
f.write_str(&padding[0..padding_size])?;
}

// In case this entry was the last on the current line,
// there is no need to print the separator and padding.
if last_in_row || current + offset >= self.cells.len() {
break;
}

// Special case if tab size was not set. Fill with spaces and separator.
if tab_size == 0 {
f.write_str(&padding[..padding_size])?;
f.write_str(&separator)?;
} else {
// Move cursor to the end of the current contents.
cursor += width;
let total_spaces = padding_size + self.options.filling.width();
// The size of \t can be inconsistent in terminal.
// Tab stops are relative to the cursor position e.g.,
// * cursor = 0, \t moves to column 8;
// * cursor = 5, \t moves to column 8 (3 spaces);
// * cursor = 9, \t moves to column 16 (7 spaces).
// Calculate the nearest \t position in relation to cursor.
let closest_tab = tab_size - (cursor % tab_size);

if closest_tab > total_spaces {
f.write_str(&padding[..total_spaces])?;
} else {
let rest_spaces = total_spaces - closest_tab;
let tabs = 1 + (rest_spaces / tab_size);
let spaces = rest_spaces % tab_size;
f.write_str(&"\t".repeat(tabs))?;
f.write_str(&padding[..spaces])?;
}

cursor += total_spaces;
}
}
f.write_str("\n")?;
Expand Down
89 changes: 88 additions & 1 deletion tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

// spell-checker:ignore underflowed

use term_grid::{Direction, Filling, Grid, GridOptions};
use term_grid::{Direction, Filling, Grid, GridOptions, DEFAULT_SEPARATOR_SIZE, SPACES_IN_TAB};

#[test]
fn no_items() {
Expand Down Expand Up @@ -238,6 +238,93 @@ fn eza_many_folders() {
assert_eq!(grid.row_count(), 20);
}

#[test]
fn filling_with_tabs() {
let grid = Grid::new(
vec![
"one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten",
"eleven", "twelve",
],
GridOptions {
direction: Direction::LeftToRight,
filling: Filling::Tabs {
spaces: DEFAULT_SEPARATOR_SIZE,
tab_size: 2,
},
width: 24,
},
);

let bits = "one\t\t two\t\t three\nfour\t five\t\t six\nseven\t eight\t nine\nten\t\t eleven\t twelve\n";
assert_eq!(grid.to_string(), bits);
assert_eq!(grid.row_count(), 4);
}

#[test]
fn padding_bigger_than_widest() {
let grid = Grid::new(
vec!["1", "2", "3"],
GridOptions {
direction: Direction::LeftToRight,
filling: Filling::Tabs {
spaces: DEFAULT_SEPARATOR_SIZE,
tab_size: SPACES_IN_TAB,
},
width: 20,
},
);

let bits = "1 2 3\n";
assert_eq!(grid.to_string(), bits);
}

#[test]
fn odd_number_of_entries() {
let cells = vec!["one", "two", "three", "four", "five"];
let grid = Grid::new(
cells.clone(),
GridOptions {
direction: Direction::LeftToRight,
filling: Filling::Spaces(2),
width: 15,
},
);

assert_eq!(grid.to_string(), "one two\nthree four\nfive\n");

let grid = Grid::new(
cells.clone(),
GridOptions {
direction: Direction::TopToBottom,
filling: Filling::Spaces(2),
width: 15,
},
);

assert_eq!(grid.to_string(), "one four\ntwo five\nthree\n");
}

#[test]
fn different_size_separator_with_tabs() {
let grid = Grid::new(
vec![
"one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten",
"eleven", "twelve",
],
GridOptions {
direction: Direction::LeftToRight,
filling: Filling::Tabs {
spaces: 4,
tab_size: 2,
},
width: 40,
},
);

let bits = "one\t\t\ttwo\t\t three\t\t four\nfive\t\tsix\t\t seven\t\t eight\nnine\t\tten\t\t eleven\t\t twelve\n";
assert_eq!(grid.to_string(), bits);
}

// These test are based on the tests in uutils ls, to ensure we won't break
// it while editing this library.
mod uutils_ls {
Expand Down
Loading