chore(table): cleanup docs and builder methods #638
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joshka
requested review from
orhun,
mindoodoo,
sayanarijit and
kdheepak
as code owners
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #638 +/- ##
=======================================
+ Coverage 91.0% 91.3% +0.3%
=======================================
Files 42 42
Lines 12808 12992 +184
=======================================
+ Hits 11656 11868 +212
+ Misses 1152 1124 -28 ☔ View full report in Codecov by Sentry. |
❯ cargo public-api diff HEAD~..HEAD
Documenting ratatui v0.24.0 (/Users/joshka/local/ratatui)
Finished dev [unoptimized + debuginfo] target(s) in 0.51s
Documenting ratatui v0.24.0 (/Users/joshka/local/ratatui)
Finished dev [unoptimized + debuginfo] target(s) in 0.49s
Removed items from the public API
=================================
-impl ratatui::widgets::HighlightSpacing
-pub fn ratatui::widgets::HighlightSpacing::should_add(&self, selection_state: bool) -> bool
Changed items in the public API
===============================
(none)
Added items to the public API
=============================
+pub fn ratatui::widgets::Cell<'a>::content<T>(self, content: T) -> Self where T: core::convert::Into<ratatui::text::Text<'a>>
+pub fn ratatui::widgets::Cell<'a>::new<T>(content: T) -> Self where T: core::convert::Into<ratatui::text::Text<'a>>
+pub fn ratatui::widgets::Row<'a>::cells<T>(self, cells: T) -> Self where T: core::iter::traits::collect::IntoIterator, <T as core::iter::traits::collect::IntoIterator>::Item: core::convert::Into<ratatui::widgets::Cell<'a>>
+pub fn ratatui::widgets::Table<'a>::rows<T>(self, rows: T) -> Self where T: core::iter::traits::collect::IntoIterator<Item = ratatui::widgets::Row<'a>>
+pub fn ratatui::widgets::TableState::new() -> Self
+pub fn ratatui::widgets::TableState::selected_mut(&mut self) -> &mut core::option::Option<usize> |
|
|
|
Note the following is a breaking change, but its highly unlikely to be one that would impact any user.
|
Table docs rendered
|
Row docs rendered
|
Cell docs rendered
|
TableState docs rendered
|
This was referenced Nov 21, 2023
Did you not mark this PR as breaking change purposefully ? |
|
I really like what you did for the documentation of the Something like :
What do you think about trying to document most widget in a similar fashion ? |
Yep. It's an internal detail that was added fairly recently, and has no reasonable code that would call it as it supports one specific case of rendering. Pragmatically I think we can probably not worry about it. |
There's an issue that tracks doc requirements. See #386 |
9 tasks
Valentin271
reviewed
Nov 26, 2023
Valentin271
reviewed
Nov 26, 2023
|
Thanks for the review @Valentin271. I think I've addressed all your feedback - let me know if there's anything I missed. |
|
Proposed commit message for changelog: feat(table): add builder methods and cleanup docs
|
|
Looks great! 👍 There's just a typo here:
|
Thanks - updated |
|
Updated the fluent setter docs in the same way as mentioned in the comments for #655 |
Valentin271
approved these changes
Dec 4, 2023
|
Needs to be manually rebased before merging |
Valentin271
reviewed
Dec 6, 2023
joshka
force-pushed
the
table-cleanup
branch
from
6db4402
to
15ea28c
Compare
--- table-cleanup-main.txt 2023-12-09 18:29:45
+++ table-cleanup.txt 2023-12-09 18:29:30
@@ -1,4 +1,7 @@
fn cell_can_be_stylized() {
+fn cell_content() {
+fn cell_new() {
+fn cell_style() {
fn ensure_percentages_less_than_100(widths: &[Constraint]) {
fn from(content: T) -> Cell<'a> {
fn get_columns_widths(&self, max_width: u16, selection_width: u16) -> Vec<(u16, u16)> {
@@ -10,24 +13,50 @@
fn min_constraint() {
fn percentage_constraint() {
fn ratio_constraint() {
+fn render(&self, buf: &mut Buffer, area: Rect) {
fn render(mut self, area: Rect, buf: &mut Buffer, state: &mut Self::State) {
fn render(self, area: Rect, buf: &mut Buffer) {
-fn render_cell(buf: &mut Buffer, cell: &Cell, area: Rect) {
+fn row_bottom_margin() {
fn row_can_be_stylized() {
+fn row_cells() {
+fn row_height() {
+fn row_new() {
+fn row_style() {
fn set_style(self, style: Style) -> Self::Item {
fn set_style(self, style: Style) -> Self::Item {
fn set_style(self, style: Style) -> Self::Item {
fn style(&self) -> Style {
fn style(&self) -> Style {
fn style(&self) -> Style {
+fn table_block() {
fn table_can_be_stylized() {
+fn table_column_spacing() {
+fn table_header() {
+fn table_highlight_spacing() {
+fn table_highlight_style() {
+fn table_highlight_symbol() {
fn table_invalid_percentages() {
+fn table_new() {
+fn table_rows() {
+fn table_state_new() {
+fn table_state_offset() {
+fn table_state_offset_mut() {
+fn table_state_select() {
+fn table_state_select_none() {
+fn table_state_selected() {
+fn table_state_selected_mut() {
+fn table_state_with_offset() {
+fn table_state_with_selected() {
+fn table_widths() {
fn test(
fn test_render_table_with_alignment() {
fn total_height(&self) -> u16 {
fn underconstrained() {
fn widths_conversions() {
+impl Cell<'_> {
impl HighlightSpacing {
+impl Row<'_> {
+impl Table<'_> {
impl TableState {
impl<'a, T> From<T> for Cell<'a>
impl<'a> Cell<'a> {
@@ -42,26 +71,32 @@
pub enum HighlightSpacing {
pub fn block(mut self, block: Block<'a>) -> Self {
pub fn bottom_margin(mut self, margin: u16) -> Self {
+pub fn cells<T>(mut self, cells: T) -> Self
pub fn column_spacing(mut self, spacing: u16) -> Self {
+pub fn content<T>(mut self, content: T) -> Self
pub fn header(mut self, header: Row<'a>) -> Self {
pub fn height(mut self, height: u16) -> Self {
pub fn highlight_spacing(mut self, value: HighlightSpacing) -> Self {
pub fn highlight_style(mut self, highlight_style: Style) -> Self {
pub fn highlight_symbol(mut self, highlight_symbol: &'a str) -> Self {
+pub fn new() -> Self {
pub fn new<R, C>(rows: R, widths: C) -> Self
pub fn new<T>(cells: T) -> Self
+pub fn new<T>(content: T) -> Self
pub fn offset(&self) -> usize {
pub fn offset_mut(&mut self) -> &mut usize {
+pub fn rows<T>(mut self, rows: T) -> Self
pub fn select(&mut self, index: Option<usize>) {
pub fn selected(&self) -> Option<usize> {
-pub fn should_add(&self, selection_state: bool) -> bool {
+pub fn selected_mut(&mut self) -> &mut Option<usize> {
pub fn style(mut self, style: Style) -> Self {
pub fn style(mut self, style: Style) -> Self {
pub fn style(mut self, style: Style) -> Self {
-pub fn widths<I, C>(mut self, widths: I) -> Self
+pub fn widths<I>(mut self, widths: I) -> Self
pub fn with_offset(mut self, offset: usize) -> Self {
pub fn with_selected(mut self, selected: Option<usize>) -> Self {
pub struct Cell<'a> {
pub struct Row<'a> {
pub struct Table<'a> {
pub struct TableState {
+pub(crate) fn should_add(&self, has_selection: bool) -> bool { |
--- table-unsorted-main.txt 2023-12-09 18:36:28
+++ table-unsorted.txt 2023-12-09 18:36:18
@@ -1,55 +1,64 @@
-pub struct Cell<'a> {
-impl<'a> Cell<'a> {
- pub fn style(mut self, style: Style) -> Self {
-impl<'a, T> From<T> for Cell<'a>
- fn from(content: T) -> Cell<'a> {
-impl<'a> Styled for Cell<'a> {
- fn style(&self) -> Style {
- fn set_style(self, style: Style) -> Self::Item {
+pub struct Table<'a> {
pub struct Row<'a> {
-impl<'a> Row<'a> {
- pub fn new<T>(cells: T) -> Self
- pub fn height(mut self, height: u16) -> Self {
- pub fn style(mut self, style: Style) -> Self {
- pub fn bottom_margin(mut self, margin: u16) -> Self {
- fn total_height(&self) -> u16 {
-impl<'a> Styled for Row<'a> {
- fn style(&self) -> Style {
- fn set_style(self, style: Style) -> Self::Item {
+pub struct Cell<'a> {
pub enum HighlightSpacing {
-impl HighlightSpacing {
- pub fn should_add(&self, selection_state: bool) -> bool {
-pub struct Table<'a> {
+pub struct TableState {
impl<'a> Table<'a> {
pub fn new<R, C>(rows: R, widths: C) -> Self
- pub fn block(mut self, block: Block<'a>) -> Self {
+ pub fn rows<T>(mut self, rows: T) -> Self
pub fn header(mut self, header: Row<'a>) -> Self {
- pub fn widths<I, C>(mut self, widths: I) -> Self
+ pub fn widths<I>(mut self, widths: I) -> Self
+ pub fn column_spacing(mut self, spacing: u16) -> Self {
+ pub fn block(mut self, block: Block<'a>) -> Self {
pub fn style(mut self, style: Style) -> Self {
- pub fn highlight_symbol(mut self, highlight_symbol: &'a str) -> Self {
pub fn highlight_style(mut self, highlight_style: Style) -> Self {
+ pub fn highlight_symbol(mut self, highlight_symbol: &'a str) -> Self {
pub fn highlight_spacing(mut self, value: HighlightSpacing) -> Self {
- pub fn column_spacing(mut self, spacing: u16) -> Self {
- fn get_columns_widths(&self, max_width: u16, selection_width: u16) -> Vec<(u16, u16)> {
- fn get_row_bounds(
pub const fn segment_size(mut self, segment_size: SegmentSize) -> Self {
-fn ensure_percentages_less_than_100(widths: &[Constraint]) {
-impl<'a> Styled for Table<'a> {
- fn style(&self) -> Style {
- fn set_style(self, style: Style) -> Self::Item {
-pub struct TableState {
+impl<'a> Row<'a> {
+ pub fn new<T>(cells: T) -> Self
+ pub fn cells<T>(mut self, cells: T) -> Self
+ pub fn height(mut self, height: u16) -> Self {
+ pub fn bottom_margin(mut self, margin: u16) -> Self {
+ pub fn style(mut self, style: Style) -> Self {
+impl<'a> Cell<'a> {
+ pub fn new<T>(content: T) -> Self
+ pub fn content<T>(mut self, content: T) -> Self
+ pub fn style(mut self, style: Style) -> Self {
+impl HighlightSpacing {
+ pub(crate) fn should_add(&self, has_selection: bool) -> bool {
impl TableState {
+ pub fn new() -> Self {
+ pub fn with_offset(mut self, offset: usize) -> Self {
+ pub fn with_selected(mut self, selected: Option<usize>) -> Self {
pub fn offset(&self) -> usize {
pub fn offset_mut(&mut self) -> &mut usize {
- pub fn with_selected(mut self, selected: Option<usize>) -> Self {
- pub fn with_offset(mut self, offset: usize) -> Self {
pub fn selected(&self) -> Option<usize> {
+ pub fn selected_mut(&mut self) -> &mut Option<usize> {
pub fn select(&mut self, index: Option<usize>) {
-impl<'a> StatefulWidget for Table<'a> {
- fn render(mut self, area: Rect, buf: &mut Buffer, state: &mut Self::State) {
-fn render_cell(buf: &mut Buffer, cell: &Cell, area: Rect) {
impl<'a> Widget for Table<'a> {
fn render(self, area: Rect, buf: &mut Buffer) {
+impl<'a> StatefulWidget for Table<'a> {
+ fn render(mut self, area: Rect, buf: &mut Buffer, state: &mut Self::State) {
+impl Table<'_> {
+ fn get_columns_widths(&self, max_width: u16, selection_width: u16) -> Vec<(u16, u16)> {
+ fn get_row_bounds(
+fn ensure_percentages_less_than_100(widths: &[Constraint]) {
+impl Row<'_> {
+ fn total_height(&self) -> u16 {
+impl Cell<'_> {
+ fn render(&self, buf: &mut Buffer, area: Rect) {
+impl<'a, T> From<T> for Cell<'a>
+ fn from(content: T) -> Cell<'a> {
+impl<'a> Styled for Cell<'a> {
+ fn style(&self) -> Style {
+ fn set_style(self, style: Style) -> Self::Item {
+impl<'a> Styled for Row<'a> {
+ fn style(&self) -> Style {
+ fn set_style(self, style: Style) -> Self::Item {
+impl<'a> Styled for Table<'a> {
+ fn style(&self) -> Style {
+ fn set_style(self, style: Style) -> Self::Item {
mod tests {
fn table_invalid_percentages() {
fn widths_conversions() {
@@ -67,3 +76,33 @@
fn table_can_be_stylized() {
fn highlight_spacing_to_string() {
fn highlight_spacing_from_str() {
+ mod table {
+ fn table_new() {
+ fn table_widths() {
+ fn table_rows() {
+ fn table_column_spacing() {
+ fn table_block() {
+ fn table_header() {
+ fn table_highlight_style() {
+ fn table_highlight_symbol() {
+ fn table_highlight_spacing() {
+ mod row {
+ fn row_new() {
+ fn row_cells() {
+ fn row_height() {
+ fn row_bottom_margin() {
+ fn row_style() {
+ mod cell {
+ fn cell_new() {
+ fn cell_content() {
+ fn cell_style() {
+ mod table_state {
+ fn table_state_new() {
+ fn table_state_with_offset() {
+ fn table_state_with_selected() {
+ fn table_state_offset() {
+ fn table_state_offset_mut() {
+ fn table_state_selected() {
+ fn table_state_selected_mut() {
+ fn table_state_select() {
+ fn table_state_select_none() { |
|
I definitely should have split this PR into two PRs - refactor, then another one that implements the new parts / makes changes. It may be that it's too difficult to reasonably review this. In which case I will redo it as 2 PRs. |
Splitting small commits for the different steps would also make your process less painful. especially when you decide to split the PR and you can just cherry-pick the specific commit out. I tend to maintain a commit for every single block field (like function, struct...), so if one day I have to make a huge rebase, the conflicts diff would be easy to read. After the PR review, I'd finally squash them together. I was advised of this process by my old colleagues. Hope you find it helpful. |
- Refactor the `table` modul for better top to bottom readability by putting types first and arranging them in a logical order (Table, Row, Cell, other). - Adds new methods for: - `Table::rows` - `Row::cells` - `Cell::new` - `Cell::content` - `TableState::new` - `TableState::selected_mut` - Makes `HighlightSpacing::should_add` pub(crate) since it's an internal detail. - Adds tests for all the new methods and simple property tests for all the other setter methods.
joshka
force-pushed
the
table-cleanup
branch
from
15ea28c
to
b73e192
Compare
Valentin271
requested changes
Dec 13, 2023
Valentin271
approved these changes
Dec 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor the
tablemodule to for better top to bottom readability byputting types first and arranging them in a logical order (Table, Row,
Cell, other).
Adds new methods for:
Table::rowsRow::cellsCell::newCell::contentTableState::newTableState::selected_mutMakes
HighlightSpacing::should_addpub(crate) since it's an internaldetail.
Adds tests for all the new methods and simple property tests for all
the other builder style methods.