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

Tables print with middle lines #43

Closed
abreis opened this issue Aug 5, 2021 · 6 comments
Closed

Tables print with middle lines #43

abreis opened this issue Aug 5, 2021 · 6 comments

Comments

@abreis
Copy link

abreis commented Aug 5, 2021

Using presets::ASCII_BORDERS_ONLY, whose sample table is:

+---------------+
| Hello   there |
+===============+
| a       b     |
| c       d     |
+---------------+

I'm instead getting an empty middle row between each actual row:

+-------------------------------+
| timestamp   one   two   three |
+===============================+
| 2001000     12    15    19    |
|                               |
| 2001001     22    25    29    |
|                               |
| 2001002     32    35    39    |
+-------------------------------+

This is with v4.0.1. Is this a bug? Or some row padding section I'm not seeing?

Thanks!

@abreis
Copy link
Author

abreis commented Aug 5, 2021

Same with presets::ASCII_HORIZONTAL_BORDERS_ONLY.

Sample:

---------------
 Hello   there
===============
 a       b
 c       d
---------------

Actual output:

-------------------------------
 timestamp   one   two   three 
===============================
 2001000     12    15    19    
-------------------------------
 2001001     22    25    29    
-------------------------------
 2001002     32    35    39    
-------------------------------

@Nukesor
Copy link
Owner

Nukesor commented Aug 6, 2021

Ok.

The first one actually seems to be broken. The newline in between the horizontal rows isn't supposed to be there.

However, in the presets::ASCII_HORIZONTAL_BORDERS_ONLY, the sample is actually wrong.
Looking at it, the naming for those presets is kind of confusing and probably not accurate.

I guess the BORDERS part could simply be removed from the name for that preset?

@abreis
Copy link
Author

abreis commented Aug 6, 2021

Ah, right, the ASCII_HORIZONTAL_BORDERS_ONLY is effectively ASCII_MARKDOWN (no lines between rows, no surrounding borders).

Yes, I think removing 'BORDERS' would be enough for that preset, plus fixing the sample (they're really useful!). That just leaves finding out what's causing the newlines in the first example.

@Nukesor
Copy link
Owner

Nukesor commented Aug 7, 2021

Ok. The preset samples were indeed a little imprecise.

I rewrote the tests to actually use the sample text from the style/presets.rs module. This should make things a little safer and more consistent.
Furthermore, I'll keep the presets::ASCII_BORDERS_ONLY as it is for now, the preset sample should soon be correct.
The current preset is probably used by quite a few people and this would break backward compatibility.
This preset is also much easier to read. Without horizontal lines, it's super hard to differentiate between rows.

Anyhow, I'm open for a new preset!
I don't have a good name for it yet.

  +/// Just like ASCII_BORDERS_ONLY, but without horizontal middle lines.
  +///
  +/// ```text
  +/// +---------------+
  +/// | Hello   there |
  +/// +===============+
  +/// | a       b     |
  +/// | c       d     |
  +/// +---------------+
  +/// ```
  +pub const ASCII_BORDERS_ONLY_WITHOUT_HORIZONTAL_DELIMITER: &str = "||--+==+     --++++";

In case you don't need it to be added to the crate, you can just take the preset string from above and import it in your table like this:

table.load_preset("||--+==+     --++++");

@abreis
Copy link
Author

abreis commented Aug 9, 2021

I agree with not breaking horizontal compatibility, and the current presets are fine as they are.

I do think a new preset would be nice to have. Although harder to read, more compact presets are very useful when one expects to print many rows (50% fewer scrolling!).

As for naming, my suggestion would be to use COMPACT or CONDENSED, e.g. ASCII_BORDERS_ONLY_CONDENSED.

Thanks again!

@Nukesor
Copy link
Owner

Nukesor commented Aug 9, 2021

I like the CONDENSED name.
Implemented in 2f7a1ea. I'll soon release a new minor version :)

@Nukesor Nukesor closed this as completed Aug 9, 2021
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

No branches or pull requests

2 participants