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

docs(examples): simplify the barchart example #1079

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

joshka
Copy link
Member

@joshka joshka commented May 2, 2024

  • Simplify the rendering functions
  • Fix several clippy lints that were marked as allowed
  • Extract a common.rs for terminal initialization and error handling

- Simplify the rendering functions
- Fix several clippy lints that were marked as allowed
- Extract a common.rs for terminal initialization and error handling
examples/barchart.rs Outdated Show resolved Hide resolved
@joshka
Copy link
Member Author

joshka commented May 15, 2024

I made a bunch of changes:

  • removed the prelude
  • revamped the way the data for the companies was stored to simplify the conversion into bars (i.e. it's stored as months containing revenues for each company rather than companies containing revenues for each month
  • moved a bunch of things around
  • tightened up pretty much everything
  • added comments where they would normally be needed.

@joshka joshka requested a review from EdJoPaTo May 15, 2024 07:53
@joshka
Copy link
Member Author

joshka commented May 15, 2024

The idea behind this is to do similar changes to all the examples, and to make them take this form.

examples/common.rs Outdated Show resolved Hide resolved
examples/common.rs Outdated Show resolved Hide resolved
examples/barchart.rs Show resolved Hide resolved
examples/barchart.rs Outdated Show resolved Hide resolved
examples/barchart.rs Show resolved Hide resolved
examples/barchart.rs Outdated Show resolved Hide resolved
Move constants closer to their usage
add an exit field to app
extract methods from the main loop to make the flow clearer
move update handling to use a field on app instead of a variable in the loop
rename last_tick to last_update to make it clearer what it controls
@joshka
Copy link
Member Author

joshka commented May 16, 2024

fix: tweaks

  • Move constants closer to their usage
  • add an exit field to app
  • extract methods from the main loop to make the flow clearer
  • move update handling to use a field on app instead of a variable in the loop
  • rename last_tick to last_update to make it clearer what it controls

@EdJoPaTo
Copy link
Member

Given that there are other Repos with widget examples… how about a ratatui feature for an example base? Other widgets in other repos can also use them as a common base to demonstrate their widgets then.

@joshka
Copy link
Member Author

joshka commented May 16, 2024

Given that there are other Repos with widget examples… how about a ratatui feature for an example base? Other widgets in other repos can also use them as a common base to demonstrate their widgets then.

An example is just an app.
Adding code to make writing examples easy is adding code to make writing apps easy.
There shouldn't be an "example base", there should be functions that make using common parts of ratatui easy. Those changes are more difficult than cleaning up existing code to make it easier for new users to understand.

@EdJoPaTo
Copy link
Member

Given that there are other Repos with widget examples… how about a ratatui feature for an example base? Other widgets in other repos can also use them as a common base to demonstrate their widgets then.

An example is just an app. Adding code to make writing examples easy is adding code to make writing apps easy. There shouldn't be an "example base", there should be functions that make using common parts of ratatui easy. Those changes are more difficult than cleaning up existing code to make it easier for new users to understand.

This creates a common example base. My point is to make this common.rs reusable for other repos too, not just examples within the ratatui repo. That way they can also be more consistent.

@joshka
Copy link
Member Author

joshka commented May 16, 2024

I think it might be better to incorporate that file in each example instead as it makes each example self contained as a single file.

Publishing public code locks you into supporting it and not making changes that would break code that uses it. I'd prefer this code to be copied instead of referenced.

@joshka joshka requested a review from EdJoPaTo May 25, 2024 23: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.

None yet

3 participants