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.

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

Rebased version of #125 #128

Merged
merged 8 commits into from
Feb 17, 2024
Merged

Rebased version of #125 #128

merged 8 commits into from
Feb 17, 2024

Conversation

jugglerchris
Copy link
Owner

No description provided.

When the raw option is set, subrenderer for cells in tables use the full
available width and ensure text within a cell is next to eachother in the
rendered text.
Raw mode additionally does not draw table boundaries.

The implementation of this must switch the iteration order from first
iterating over same lines of adjacent cells to first iterating over the cells and
then over every line of that cell.
@jugglerchris jugglerchris mentioned this pull request Feb 10, 2024
@@ -169,6 +169,18 @@ impl<T: Debug + Eq + PartialEq + Clone + Default> TaggedLine<T> {
true
}

/// Return true if the line is non-empty
Copy link
Owner Author

@jugglerchris jugglerchris Feb 10, 2024

Choose a reason for hiding this comment

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

I think the comment is backwards.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed.

@jugglerchris
Copy link
Owner Author

@sftse This looks good to me. Do you have any concerns about how it works now?

I don't quite get how this works with deeply nested tables (see deeply_nested test), but think this could benefit from your input at this stage.

Do you mean the "vertical row" feature? (If columns don't fit next to each other, fall back to putting them on top of each other with "////" as a border to try to show they belong side by side)

That's a good point, thinking about it. Maybe raw mode doesn't need much more than:

index 6959faf..0bd346d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1834,7 +1834,7 @@ fn render_table_tree<T: Write, D: TextDecorator>(
         + col_sizes.len().saturating_sub(1);
     let width = renderer.width();
 
-    let vert_row = min_size > width || width == 0;
+    let vert_row = renderer.options.raw || (min_size > width || width == 0);
 
     let mut col_widths: Vec<usize> = if !vert_row {
         col_sizes

along with !draw_borders.

@jugglerchris
Copy link
Owner Author

Ok, I've updated to use vertical rows instead of a separate loop over rows.
I've also removed some of the if raw tests when making subrenderers - in most cases they come for free because the prefixes are zero length from TrivialDecorator. There was one case (Dd) with a hard-coded prefix though.
Finally, the output from the test is slightly different - there's no trailing whitespace (which was there because normal table columns are padded, but that's not needed for the "vertical" columns). I think that's probably better.

@sftse
Copy link
Contributor

sftse commented Feb 13, 2024

I can't push to this branch, but this idea turned out great. I reverted even more of my previous changes, see #129

@jugglerchris jugglerchris merged commit e6e4015 into main Feb 17, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants