-
Notifications
You must be signed in to change notification settings - Fork 7
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
Changes from all commits
97360e1
ab7e5d4
30594c9
a836447
5004ad8
01ba349
d4989f3
6962463
cfec022
39b377e
23bf389
ef8ffc4
e705bdf
89f0d98
c987084
7b88c50
b3382b5
1e61c40
a137979
95a9aa8
afd508a
16e2a0d
988bcc2
12105f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,12 @@ | |
use ansi_width::ansi_width; | ||
use std::fmt; | ||
|
||
/// Number of spaces in one \t. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: 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 { | ||
|
@@ -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, | ||
} | ||
} | ||
} | ||
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder whether using There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is indeed what I would suggest, but the
It would functionally be very similar to what you have, but it would make it very clear that See https://doc.rust-lang.org/stable/std/num/type.NonZeroUsize.html for more info on that type. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! You can just do a There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 => {
// ...
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could write that with an 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 | ||
|
@@ -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; | ||
|
||
|
@@ -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")?; | ||
|
There was a problem hiding this comment.
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
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!
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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!