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

upgrade papergrid to a later version of unicode-width #426

Closed
fdncred opened this issue Sep 6, 2024 · 11 comments
Closed

upgrade papergrid to a later version of unicode-width #426

fdncred opened this issue Sep 6, 2024 · 11 comments
Labels
question Further information is requested

Comments

@fdncred
Copy link
Contributor

fdncred commented Sep 6, 2024

@zhiburt we're running into issues with nushell. we can't upgrade to crossterm 28.1 and ratatui 28.1. I think this is because tabled has a dep on papergrid which locks the unicode-width dep at 0.1.11. Would you mind updating papergrid/tabled with a later version?

unicode-width = "=0.1.11"

@zhiburt
Copy link
Owner

zhiburt commented Sep 7, 2024

Yep....................
..........................
..........................

Something I must do.
I'll ponder about it in short, hopefully.

To be honest I haven't had a good idea how to fix it.
Though wasn't thinking a lot :)

ref #423

@zhiburt zhiburt added the question Further information is requested label Sep 7, 2024
@pycui
Copy link

pycui commented Oct 2, 2024

+1 on this, it's causing version conflicts since unicode-width is widely used in other crates. Appreciate if we can fix it

@zhiburt
Copy link
Owner

zhiburt commented Oct 10, 2024

Hi there

What do you think if I just fork unicode-width and that's it? 😄
IDK why I haven't thought about it at first.

@zhiburt
Copy link
Owner

zhiburt commented Oct 10, 2024

Somewhat I started to think that maybe this nushell/nushell#13088 (comment) was not an actual issue.

At this point.
Seems like I haven't investigated it deep enough.

@fdncred
Copy link
Contributor Author

fdncred commented Oct 10, 2024

maybe there's some other crate other than unicode-width that you can use? I'm not sure such a thing exists. It's a pain not being able to update ratatui and other deps. If you fork unicode-width, you could rename it to tabled-unicode-width so that it doesn't conflict with the other one maybe?

@zhiburt
Copy link
Owner

zhiburt commented Oct 11, 2024

If you fork unicode-width, you could rename it to tabled-unicode-width so that it doesn't conflict with the other one maybe?

Exactly
And it's a 5 minute job.

I guess It's a good solution unless I figure it out completely :(

@fdncred
Copy link
Contributor Author

fdncred commented Oct 11, 2024

I think you should go ahead and do it. The worst that can happen is that you have to revert or change it. It's just software, it can be changed again.

@joshtriplett
Copy link
Contributor

I've read through the entirety of unicode-rs/unicode-width#55 and unicode-rs/unicode-width#64 , and as far as I can tell, I think the new behavior of unicode-width is more correct for tabled's purposes (e.g. handling the width of emoji correctly), as long as tabled never feeds control characters to unicode-width. (For control characters, unicode-width will guess that the terminal might choose to render a character-sized replacement character, but some terminals may not render such characters at all; there's no consistent answer here other than to not handle control characters. I think it'd be reasonable to tell people to just not put control characters into table cells and expect reasonable results.)

Rather than forking unicode-width to preserve the old behavior (which was incorrect for, among other things, many composed emoji using ZWJ), I think it would make sense to port to the new behavior of unicode-width and have a dependency on that version of unicode-width.

@joshtriplett
Copy link
Contributor

I've published a PR at #430 to use current unicode-width and improve width handling.

@joshtriplett
Copy link
Contributor

I think this issue can now be closed.

@zhiburt
Copy link
Owner

zhiburt commented Nov 22, 2024

Must be released;
Eventually 😅

tabled = "=0.17.0"

@zhiburt zhiburt closed this as completed Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants