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

feat(widgets): Collect iterator of Row into Table #774

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

Lunderberg
Copy link
Contributor

A follow-up from #755, allowing any iterator whose item is convertible into Row to be collected into a Table.

Copy link

codecov bot commented Jan 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f29c73f) 92.4% compared to head (afb6689) 92.4%.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #774   +/-   ##
=====================================
  Coverage   92.4%   92.4%           
=====================================
  Files         57      57           
  Lines      15000   15024   +24     
=====================================
+ Hits       13866   13890   +24     
  Misses      1134    1134           

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

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.

LGTM, just had a few comments. Only the table widths one needs to be checked and addressed if needed. (On mobile rn, hard to check for me)

src/widgets/table/table.rs Outdated Show resolved Hide resolved
src/widgets/table/table.rs Outdated Show resolved Hide resolved
src/widgets/table/table.rs Outdated Show resolved Hide resolved
Copy link
Member

@joshka joshka left a comment

Choose a reason for hiding this comment

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

LGTM - just missing the full breaking change description.

commit message for the changelog should probably be something like

feat(table)!: accept IntoItertor<Into<Row>> for constructors

Table::new() now accepts ...
Table::from_iterator() ...

BREAKING CHANGE:
Calls to Table::new() passing rows as as an infered type
(e.g. `vec![]`) will no longer compile because the compiler
cannot to infer the type. (**reword this a bit better**)

src/widgets/table/table.rs Show resolved Hide resolved
BREAKING-CHANGES.md Show resolved Hide resolved
A follow-up from ratatui#755,
allowing any iterator whose item is convertible into `Row` to be
collected into a `Table`.

Where previously, `Table::new` accepted `IntoIterator<Item = Row>`, it
now accepts `IntoIterator<Item: Into<Row>>`.

BREAKING CHANGE:
The compiler can no longer infer the element type of the container
passed to `Table::new()`.  For example, `Table::new(vec![], widths)`
will no longer compile, as the type of `vec![]` can no longer be
inferred.
Copy link
Member

@joshka joshka left a comment

Choose a reason for hiding this comment

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

LGTM

@joshka joshka dismissed Valentin271’s stale review January 11, 2024 01:31

comments are resolved

@joshka joshka merged commit c69ca47 into ratatui:main Jan 11, 2024
34 checks passed
@joshka
Copy link
Member

joshka commented Jan 11, 2024

Thanks again for the PR

@Lunderberg Lunderberg deleted the table_from_iterator branch January 11, 2024 12:26
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.

3 participants