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

table produces subtraction overflow #12786

Open
fdncred opened this issue May 6, 2024 · 3 comments
Open

table produces subtraction overflow #12786

fdncred opened this issue May 6, 2024 · 3 comments
Labels
🐛 bug Something isn't working needs-triage An issue that hasn't had any proper look panic tabled Issues about our new table renderer (tabled)

Comments

@fdncred
Copy link
Collaborator

fdncred commented May 6, 2024

Describe the bug

@zhiburt While troubleshooting my emoji plugin, I noticed that it hangs if skin_tones column is shown.

How to reproduce

Alternate Repro

devyn just figured out we could reproduce the error with emoji --list | select emoji name skin_tones | save emoji.json and then try to open emoji.json and you get the same error.
emoji.json

Original Repro (harder to do)

  1. Build nushell
  2. Build nu_plugin_emoji https://github.com/fdncred/nu_plugin_emoji
  3. in nushell do `plugin add /path/to/nu_plugin_emoji
  4. quit out of nushell and start it back up again
  5. now you can do emoji :smirk: and see that it works
  6. now try emoji --list, if your terminal window is small and not showing the skin_tones column, it should work.
  7. To see it fail do emoji --list | select emoji name skin_tones to force skin_tones column to show. It should hang and not show anything.

Further testing
This works emoji --list | where shortcodes =~ wave so it's not just about the skin_tones column but some specific data.

@devyn has been helping me troubleshoot and produced this

#0  0x00005b1e18f535f1 in <alloc::string::String as core::fmt::Write>::write_char ()
#1  0x00005b1e1a4b28ff in papergrid::grid::peekable::print_indent ()
#2  0x00005b1e1a4b267d in papergrid::grid::peekable::print_line ()
#3  0x00005b1e1a4b3377 in papergrid::grid::peekable::print_cell_line ()
#4  0x00005b1e1a4b0e6e in papergrid::grid::peekable::build_grid ()
#5  0x00005b1e1a4b214e in papergrid::grid::peekable::print_grid ()
#6  0x00005b1e1a4b6f46 in papergrid::grid::peekable::PeekableGrid<R,G,D,C>::build ()
#7  0x00005b1e1a4c9ee9 in <tabled::tables::table::Table as core::fmt::Display>::fmt ()
#8  0x00005b1e18f54916 in nu_table::table::NuTable::draw ()

and this by building in debug mode

proj nushell/nushell: emoji --list | select name skin_tones                                                                                                                                                               [⎇  main] 2024-05-06 15:47:27
Error:   × Main thread panicked.
  ├─▶ at /home/devyn/.cargo/registry/src/index.crates.io-6f17d22bba15001f/papergrid-0.10.0/src/grid/peekable.rs:715:16
  ╰─▶ attempt to subtract with overflow
  help: set the `RUST_BACKTRACE=1` environment variable to display a backtrace.

Expected behavior

Show all the columns

Screenshots

No response

Configuration

key value
version 0.93.1
major 0
minor 93
patch 1
branch main
commit_hash e879d4e
build_os macos-aarch64
build_target aarch64-apple-darwin
rust_version rustc 1.77.2 (25ef9e3d8 2024-04-09)
rust_channel 1.77.2-aarch64-apple-darwin
cargo_version cargo 1.77.2 (e52e36006 2024-03-26)
build_time 2024-05-05 13:28:43 -05:00
build_rust_channel release
allocator mimalloc
features dataframe, default, sqlite, system-clipboard, trash, which
installed_plugins bexpand, bg, emoji, explore, file, formats, from_parquet, gstat, highlight, image, inc, json_path, jwalk, md, plist, pnet, polars, port_list, port_scan, qr_maker, query, query_git, regex, str_similarity, stress_internals

Additional context

No response

@fdncred fdncred added 🐛 bug Something isn't working tabled Issues about our new table renderer (tabled) needs-triage An issue that hasn't had any proper look labels May 6, 2024
@zhiburt
Copy link
Contributor

zhiburt commented May 7, 2024

Hi there.

Very interesting catch;

So I did reproduce, and located issue, that is related to wrapping.
I'll try to fix in the nearest, let you know when done (maybe tommorow will see)

@zhiburt
Copy link
Contributor

zhiburt commented May 7, 2024

Kind of related;
I was thinking recently about this emoji context.

We probably could provide a user predefined set of widths estimations per terminal.
*In a sense of nushell we could peak one automatically or from config.

What I mean is just define a map of space UTF-8 symbol takes per terminal, and relay on this when making calculation of space rather then default approach.
We could do not per symbol but per range etc.

Yes it would be it a bit slower just to check if the symbol is in the map.
But if it would make it more accurate why not.
And it could be optional via a flag in table command.

It might has not a lot of sense,
Just a thought I got a few days back.

Anyhow.
Take care.

@fdncred
Copy link
Collaborator Author

fdncred commented May 7, 2024

Hey @zhiburt! Thanks for looking into this. I'm wondering if fish-shell's https://github.com/fish-shell/fish-shell/blob/master/src/widecharwidth/widechar_width.rs would help here. I'm not sure if it covers the entire utf-8 symbol space but it may be worth a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working needs-triage An issue that hasn't had any proper look panic tabled Issues about our new table renderer (tabled)
Projects
None yet
Development

No branches or pull requests

3 participants