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

fix: Render table with overflow #685

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

YeungKC
Copy link
Contributor

@YeungKC YeungKC commented Dec 12, 2023

This PR aims to fix an issue in the render_cell function that could lead to an index out of range error. The problem was caused by incorrect positioning of the x coordinate in the original code.

Fixes #684

Copy link

codecov bot commented Dec 12, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d19b266) 90.9% compared to head (df40caf) 91.0%.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #685   +/-   ##
=====================================
  Coverage   90.9%   91.0%           
=====================================
  Files         42      42           
  Lines      12795   12808   +13     
=====================================
+ Hits       11643   11656   +13     
  Misses      1152    1152           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@YeungKC YeungKC changed the title fix(Table): render table with overflow fix:Render table with overflow Dec 12, 2023
@YeungKC YeungKC changed the title fix:Render table with overflow fix: Render table with overflow Dec 12, 2023
@YeungKC YeungKC force-pushed the bugfix-overflow branch 3 times, most recently from 3b6f06b to 7c16d33 Compare December 12, 2023 12:06
Copy link
Sponsor Member

@orhun orhun left a comment

Choose a reason for hiding this comment

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

Can confirm that the panic is fixed with this. Not sure if we need more tests / if there is any edge cases though. General approval +

@YeungKC
Copy link
Contributor Author

YeungKC commented Dec 12, 2023

I'm not very know this lib, but I think, normally, only an empty string would trigger that issue.

Copy link
Member

@Valentin271 Valentin271 left a comment

Choose a reason for hiding this comment

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

I think that's enough test. I agree with @YeungKC, only an empty string, right aligned and with no border can trigger this.

@Valentin271 Valentin271 merged commit aaeba27 into ratatui-org:main Dec 12, 2023
33 checks passed
@YeungKC YeungKC deleted the bugfix-overflow branch December 12, 2023 19:52
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.

Debug assertion on too-wide, empty, right-aligned table cell
3 participants