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

How to format special characters correctly? #44

Closed
PsiACE opened this issue Aug 6, 2021 · 11 comments
Closed

How to format special characters correctly? #44

PsiACE opened this issue Aug 6, 2021 · 11 comments

Comments

@PsiACE
Copy link

PsiACE commented Aug 6, 2021

databendlabs/databend#1323

preset: table.load_preset("||--+-++| ++++++");

Cell::new(format!("{}", "✅ ".blue())); // error, missing spaces for padding. `.blue()` comes from `colored`

error-pretty-table

Cell::new(format!("{}", "✅ "));  // error, it looks like there is an extra blank character 

wrong-pretty-table

Cell::new(format!("{}", "☑")); or Cell::new("☑").fg(Color::Green); // ok, `CellAlignment::Center`, but i want ✅

pretty-table

@PsiACE
Copy link
Author

PsiACE commented Aug 6, 2021

the result of prettytable-rs , please just consider ✅

old-pretty-table

@Nukesor
Copy link
Owner

Nukesor commented Aug 6, 2021

  1. the .blue() call is nothing that's handled by comfy-table right now. .blue() adds an ASCII escape sequence to the string, which is just interpreted as additional characters. As those characters are no longer shown in the table after being sent to the terminal, it looks like that row is way too short. This should be solved with Comfy-tables internal coloring mechanisms.
  2. The second one is actually a bug. That's the way it should be and this shouldn't happen. I'm investigating.

@Nukesor
Copy link
Owner

Nukesor commented Aug 6, 2021

Ok. Maybe I need some information about how those characters are printed.

Can it be, that there are utf8-characters that actually take up two spaces in a mono-font text?
I just wrote a test and this seems to be the case o.O. When printing (?) it simply adds another space behind it, resulting in two characters, even though it's a single character in the code...

@Nukesor
Copy link
Owner

Nukesor commented Aug 6, 2021

Prettytables uses the unicode-width crate, which seems to be the way to go.

However, even the unicode-width docs say, that for some emojis, the calculated width may differ from the actually displayed value...
https://docs.rs/crate/unicode-width/0.1.8

I'm not exactly sure what's the right thing to do here. I didn't know that unicode was such a mess.

@Nukesor
Copy link
Owner

Nukesor commented Aug 6, 2021

This is super annoying.

unicode-width is probably the way to go, but this behavior really destroys the consistency of this crate.

Comfy-table's whole internal logic operates under the assumption, that a single utf-8 character is actually displayed as a single character.

In most cases, this won't be a problem. However, it gets super messy in some edge-cases.
For instance, if there's basically no space left, comfy-tables falls back to a 1-space wide column.
With those utf-8 characts, this is simply not possible.

╭───╮
│   │
├╌╌╌┤
│ ⺀ │
╰───╯

As a result, the property-testing with proptest fails, as the expected behavior can no longer be enforced.
We have absolutely no control over how those characters are displayed.

I need to think about this. We might have to do some internal refactoring to properly handle these issues.
I really don't like this.

@Nukesor
Copy link
Owner

Nukesor commented Aug 6, 2021

The linked PR already fixes your issue, but the internal issues aren't addressed yet.

@Nukesor
Copy link
Owner

Nukesor commented Aug 7, 2021

Some broken things found by Proptest:

╭───╮
│   │
├╌╌╌┤
│   │
├╌╌╌┤
│   │
├╌╌╌┤
│   │
├╌╌╌┤
│   │
├╌╌╌┤
│   │
├╌╌╌┤
│ 򐮆 │
│ 򮔙 │
│
    │
│   │
│   │
│ . │
│   │
│ . │
│








 │
│ : │
│ 󀅀 │
│ ? │
│ F │
│ Ⱥ │
│ : │
│ B │
│ 򋴣 │
│  ‮ │
│ ? │
│ < │
│ 🕴 │
│ 𾣒 │
├╌╌╌┤
│ 򥟀 │
│ : │
│ " │
│ ? │
│ \ │
│ 1 │
╰───╯
╭───╮
│ 🞊 │
│  	 │
 │
│ 󅈧 │
│ 𘏴 │
│ { │
│ ? │
│ ( │
│ & │
│ Ⱥ │
╰───╯

@PsiACE
Copy link
Author

PsiACE commented Aug 7, 2021

thank you very much, recently I am a loyal user of comfy-table

@Nukesor
Copy link
Owner

Nukesor commented Aug 8, 2021

Ok.
I'm going to merge the current state of the PR.
I also think that there's currently no good or simple way to fix broken table formatting, if users decide to

  1. Have 1 character wide columns that contain multi-character utf8 symbols.

Defaulting to a multi-character column in this case would be in direct conflict with the user's wish to have a single-character column. I don't think this case should be handled. If the user requests it, they should get it.

  1. Dynamic mode is active and comfy-table falls back to 1 character wide columns due to too little space.

Checking if we should default to a multi-character column in the second case will be quite a lot of work, as the whole width-calculation algorightm has to be adjusted and this is quite a bit of complex code.

As I'm not sure on how to properly do that, this issue will be fixed in a future version.

@PsiACE
Copy link
Author

PsiACE commented Aug 8, 2021

You are right. Thank you for your work.

@Nukesor
Copy link
Owner

Nukesor commented Oct 29, 2021

Closing this in favor of a more specific follow-up issue :)

Thanks again for reporting!

@Nukesor Nukesor closed this as completed Oct 29, 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