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: add an impl of DoubleEndedIterator for Columns and Rows (#1357) #1358

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

fujiapple852
Copy link
Contributor

Closes #1357

@fujiapple852 fujiapple852 requested a review from a team as a code owner September 7, 2024 07:35
Copy link

codecov bot commented Sep 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.1%. Comparing base (3df685e) to head (4680e6e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1358   +/-   ##
=====================================
  Coverage   94.0%   94.1%           
=====================================
  Files         68      68           
  Lines      16360   16433   +73     
=====================================
+ Hits       15393   15466   +73     
  Misses       967     967           

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

src/layout/rect/iter.rs Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Sep 7, 2024

🐰 Bencher Report

Branch1358/merge
Testbedubuntu-latest

⚠️ WARNING: The following Measure does not have a Threshold. Without a Threshold, no Alerts will ever be generated!

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results
BenchmarkLatencynanoseconds (ns)
barchart/render/2048📈 view plot
⚠️ NO THRESHOLD
193,840.00
barchart/render/256📈 view plot
⚠️ NO THRESHOLD
128,390.00
barchart/render/64📈 view plot
⚠️ NO THRESHOLD
84,924.00
barchart/render_grouped/2048📈 view plot
⚠️ NO THRESHOLD
336,130.00
barchart/render_grouped/256📈 view plot
⚠️ NO THRESHOLD
138,260.00
barchart/render_grouped/64📈 view plot
⚠️ NO THRESHOLD
127,530.00
barchart/render_horizontal/2048📈 view plot
⚠️ NO THRESHOLD
160,410.00
barchart/render_horizontal/256📈 view plot
⚠️ NO THRESHOLD
79,810.00
barchart/render_horizontal/64📈 view plot
⚠️ NO THRESHOLD
73,948.00
block/render_all_feature/100x50📈 view plot
⚠️ NO THRESHOLD
11,497.00
block/render_all_feature/200x50📈 view plot
⚠️ NO THRESHOLD
19,955.00
block/render_all_feature/256x256📈 view plot
⚠️ NO THRESHOLD
94,961.00
block/render_empty/100x50📈 view plot
⚠️ NO THRESHOLD
6,324.00
block/render_empty/200x50📈 view plot
⚠️ NO THRESHOLD
12,263.00
block/render_empty/256x256📈 view plot
⚠️ NO THRESHOLD
80,872.00
buffer/empty/16📈 view plot
⚠️ NO THRESHOLD
785.56
buffer/empty/255📈 view plot
⚠️ NO THRESHOLD
224,070.00
buffer/empty/64📈 view plot
⚠️ NO THRESHOLD
13,560.00
buffer/filled/16📈 view plot
⚠️ NO THRESHOLD
797.99
buffer/filled/255📈 view plot
⚠️ NO THRESHOLD
224,090.00
buffer/filled/64📈 view plot
⚠️ NO THRESHOLD
13,240.00
buffer/with_lines/16📈 view plot
⚠️ NO THRESHOLD
16,024.00
buffer/with_lines/255📈 view plot
⚠️ NO THRESHOLD
14,936.00
buffer/with_lines/64📈 view plot
⚠️ NO THRESHOLD
16,229.00
line_render/Center/0📈 view plot
⚠️ NO THRESHOLD
4.04
line_render/Center/10📈 view plot
⚠️ NO THRESHOLD
638.80
line_render/Center/3📈 view plot
⚠️ NO THRESHOLD
314.50
line_render/Center/4📈 view plot
⚠️ NO THRESHOLD
355.68
line_render/Center/42📈 view plot
⚠️ NO THRESHOLD
837.76
line_render/Center/6📈 view plot
⚠️ NO THRESHOLD
401.73
line_render/Center/7📈 view plot
⚠️ NO THRESHOLD
452.07
line_render/Left/0📈 view plot
⚠️ NO THRESHOLD
4.05
line_render/Left/10📈 view plot
⚠️ NO THRESHOLD
595.71
line_render/Left/3📈 view plot
⚠️ NO THRESHOLD
226.74
line_render/Left/4📈 view plot
⚠️ NO THRESHOLD
245.99
line_render/Left/42📈 view plot
⚠️ NO THRESHOLD
837.64
line_render/Left/6📈 view plot
⚠️ NO THRESHOLD
388.06
line_render/Left/7📈 view plot
⚠️ NO THRESHOLD
402.34
line_render/Right/0📈 view plot
⚠️ NO THRESHOLD
4.04
line_render/Right/10📈 view plot
⚠️ NO THRESHOLD
577.63
line_render/Right/3📈 view plot
⚠️ NO THRESHOLD
289.04
line_render/Right/4📈 view plot
⚠️ NO THRESHOLD
348.85
line_render/Right/42📈 view plot
⚠️ NO THRESHOLD
835.53
line_render/Right/6📈 view plot
⚠️ NO THRESHOLD
466.08
line_render/Right/7📈 view plot
⚠️ NO THRESHOLD
526.96
list/render/16384📈 view plot
⚠️ NO THRESHOLD
1,196,800.00
list/render/2048📈 view plot
⚠️ NO THRESHOLD
328,230.00
list/render/64📈 view plot
⚠️ NO THRESHOLD
208,550.00
list/render_scroll_half/16384📈 view plot
⚠️ NO THRESHOLD
1,206,000.00
list/render_scroll_half/2048📈 view plot
⚠️ NO THRESHOLD
324,580.00
list/render_scroll_half/64📈 view plot
⚠️ NO THRESHOLD
143,660.00
paragraph/new/2048📈 view plot
⚠️ NO THRESHOLD
261,090.00
paragraph/new/64📈 view plot
⚠️ NO THRESHOLD
7,041.60
paragraph/new/65535📈 view plot
⚠️ NO THRESHOLD
8,252,000.00
paragraph/render/2048📈 view plot
⚠️ NO THRESHOLD
626,570.00
paragraph/render/64📈 view plot
⚠️ NO THRESHOLD
582,980.00
paragraph/render/65535📈 view plot
⚠️ NO THRESHOLD
1,813,700.00
paragraph/render_scroll_full/2048📈 view plot
⚠️ NO THRESHOLD
621,030.00
paragraph/render_scroll_full/64📈 view plot
⚠️ NO THRESHOLD
642,680.00
paragraph/render_scroll_full/65535📈 view plot
⚠️ NO THRESHOLD
1,801,000.00
paragraph/render_scroll_half/2048📈 view plot
⚠️ NO THRESHOLD
615,710.00
paragraph/render_scroll_half/64📈 view plot
⚠️ NO THRESHOLD
645,650.00
paragraph/render_scroll_half/65535📈 view plot
⚠️ NO THRESHOLD
1,799,900.00
paragraph/render_wrap/2048📈 view plot
⚠️ NO THRESHOLD
300,160.00
paragraph/render_wrap/64📈 view plot
⚠️ NO THRESHOLD
264,440.00
paragraph/render_wrap/65535📈 view plot
⚠️ NO THRESHOLD
1,482,700.00
paragraph/render_wrap_scroll_full/2048📈 view plot
⚠️ NO THRESHOLD
301,820.00
paragraph/render_wrap_scroll_full/64📈 view plot
⚠️ NO THRESHOLD
262,680.00
paragraph/render_wrap_scroll_full/65535📈 view plot
⚠️ NO THRESHOLD
1,472,200.00
rect_rows/rows/1024📈 view plot
⚠️ NO THRESHOLD
324.85
rect_rows/rows/16📈 view plot
⚠️ NO THRESHOLD
5.60
rect_rows/rows/65535📈 view plot
⚠️ NO THRESHOLD
20,392.00
sparkline/render/2048📈 view plot
⚠️ NO THRESHOLD
113,070.00
sparkline/render/256📈 view plot
⚠️ NO THRESHOLD
113,860.00
sparkline/render/64📈 view plot
⚠️ NO THRESHOLD
35,630.00
table/render/16384x2📈 view plot
⚠️ NO THRESHOLD
2,866,900.00
table/render/16384x4📈 view plot
⚠️ NO THRESHOLD
5,349,600.00
table/render/16384x8📈 view plot
⚠️ NO THRESHOLD
17,000,000.00
table/render/2048x2📈 view plot
⚠️ NO THRESHOLD
646,100.00
table/render/2048x4📈 view plot
⚠️ NO THRESHOLD
1,112,300.00
table/render/2048x8📈 view plot
⚠️ NO THRESHOLD
1,811,000.00
table/render/64x2📈 view plot
⚠️ NO THRESHOLD
341,980.00
table/render/64x4📈 view plot
⚠️ NO THRESHOLD
529,290.00
table/render/64x8📈 view plot
⚠️ NO THRESHOLD
618,240.00
table/render_scroll_half/16384x2📈 view plot
⚠️ NO THRESHOLD
2,878,600.00
table/render_scroll_half/16384x4📈 view plot
⚠️ NO THRESHOLD
5,757,400.00
table/render_scroll_half/16384x8📈 view plot
⚠️ NO THRESHOLD
19,158,000.00
table/render_scroll_half/2048x2📈 view plot
⚠️ NO THRESHOLD
661,170.00
table/render_scroll_half/2048x4📈 view plot
⚠️ NO THRESHOLD
1,117,300.00
table/render_scroll_half/2048x8📈 view plot
⚠️ NO THRESHOLD
1,816,300.00
table/render_scroll_half/64x2📈 view plot
⚠️ NO THRESHOLD
235,040.00
table/render_scroll_half/64x4📈 view plot
⚠️ NO THRESHOLD
346,580.00
table/render_scroll_half/64x8📈 view plot
⚠️ NO THRESHOLD
410,070.00
🐰 View full continuous benchmarking report in Bencher

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 generally.

  • We should break the use of the pub fields for 0.29.0
  • Some small ergonomics of the code for symmetry / better understanding
  • This PR should include rows() too

src/layout/rect/iter.rs Outdated Show resolved Hide resolved
src/layout/rect/iter.rs Outdated Show resolved Hide resolved
src/layout/rect/iter.rs Outdated Show resolved Hide resolved
src/layout/rect/iter.rs Outdated Show resolved Hide resolved
@fujiapple852 fujiapple852 force-pushed the feat-columns-double-ended-iter branch from b0e69c0 to 8bb01ac Compare September 7, 2024 13:30
@fujiapple852
Copy link
Contributor Author

Added DoubleEndedIterator for Rows as well, and added tests.

@fujiapple852 fujiapple852 changed the title feat: add an impl of DoubleEndedIterator for Columns (#1357) feat: add an impl of DoubleEndedIterator for Columns and Rows (#1357) Sep 7, 2024
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 (repeating for other maintainers), hold merging this closer to 0.29 - I'd guess this might be sometime in ~2-4 weeks.

src/layout/rect/iter.rs Outdated Show resolved Hide resolved
@fujiapple852 fujiapple852 force-pushed the feat-columns-double-ended-iter branch from 8bb01ac to 73aa535 Compare September 9, 2024 09:49
@orhun orhun added this to the v0.29.0 milestone Oct 2, 2024
@joshka joshka force-pushed the feat-columns-double-ended-iter branch from 73aa535 to 4680e6e Compare October 14, 2024 22:36
@joshka joshka merged commit d449147 into ratatui:main Oct 14, 2024
39 of 40 checks passed
joshka pushed a commit that referenced this pull request Oct 14, 2024
…1358)

BREAKING-CHANGE: The `pub` modifier has been removed from fields on the
`layout::rect::Columns` and `layout::rect::Rows` iterators. These fields
were not intended to be public and should not have been accessed
directly.

Fixes: #1357
joshka pushed a commit to erak/ratatui that referenced this pull request Oct 14, 2024
…atatui#1358)

BREAKING-CHANGE: The `pub` modifier has been removed from fields on the
`layout::rect::Columns` and `layout::rect::Rows` iterators. These fields
were not intended to be public and should not have been accessed
directly.

Fixes: ratatui#1357
kdheepak added a commit that referenced this pull request Oct 16, 2024
* origin/main: (21 commits)
  perf: implement size hints for `Rect` iterators (#1420)
  fix(color): fix doc test for from_hsl (#1421)
  docs: tweak readme (#1419)
  refactor(color)!: use palette types for Hsl/Hsluv conversions (#1418)
  chore(deps)!: pin unicode-width to 0.2.0 (#1403)
  feat(scrolling-regions)!: use terminal scrolling regions to stop Terminal::insert_before from flickering (#1341)
  feat!: add an impl of `DoubleEndedIterator` for `Columns` and `Rows` (#1358)
  fix(rect)!: Rect::area now returns u32 and Rect::new() no longer clamps area to u16::MAX (#1378)
  docs: fix missing breaking changes link (#1416)
  feat(line)!: impl From<Cow<str>> for Line (#1373)
  chore(deny): allow Zlib license in cargo-deny configuration (#1411)
  feat(tabs)!: allow tabs to be deselected (#1413)
  feat(color): add hsluv support (#1333)
  feat(table)!: add support for selecting column and cell (#1331)
  feat(text): improve concise debug view for Span,Line,Text,Style (#1410)
  chore: add benchmark for `Table` (#1408)
  refactor: Consistent result expected in layout tests (#1406)
  feat(logo): Add a Ratatui logo widget
  chore(deps): update rstest requirement from 0.22.0 to 0.23.0 (#1394)
  chore(deps): update octocrab requirement from 0.40.0 to 0.41.0 (#1393)
  ...
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.

Add an impl of DoubleEndedIterator for Columns and Rows
3 participants