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

chore: implement Hash common traits #381

Merged
merged 1 commit into from
Aug 11, 2023

Conversation

TieWay59
Copy link
Contributor

@TieWay59 TieWay59 commented Aug 7, 2023

Reorder the derive fields to be more consistent:

Debug, Default, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash

Hash trait won't be impl in this PR due to rust std design. If we need hash trait for f64 related structs in the future, we should consider wrap f64 into a new type.

see: #307

@TieWay59
Copy link
Contributor Author

TieWay59 commented Aug 7, 2023

If we need a hash trait for f64-related structs in the future, we should consider wrapping f64 into a new type.

This has been done by some crates like: https://github.com/reem/rust-ordered-float

But, I don't think we need such high precision in terminal UI, I believe if precision is limited, we can always use int to represent most of the data struct values.

@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

Merging #381 (1757274) into main (de9f52f) will not change coverage.
The diff coverage is 54.09%.

@@           Coverage Diff           @@
##             main     #381   +/-   ##
=======================================
  Coverage   84.99%   84.99%           
=======================================
  Files          40       40           
  Lines        8686     8686           
=======================================
  Hits         7383     7383           
  Misses       1303     1303           
Files Changed Coverage Δ
src/backend/crossterm.rs 0.00% <0.00%> (ø)
src/backend/mod.rs 33.33% <0.00%> (ø)
src/backend/termion.rs 27.46% <0.00%> (ø)
src/backend/test.rs 43.63% <0.00%> (ø)
src/layout.rs 89.18% <0.00%> (ø)
src/terminal.rs 55.73% <0.00%> (ø)
src/widgets/calendar.rs 81.06% <0.00%> (ø)
src/widgets/canvas/map.rs 0.00% <0.00%> (ø)
src/widgets/canvas/mod.rs 91.61% <0.00%> (ø)
src/widgets/clear.rs 0.00% <0.00%> (ø)
... and 21 more

Reorder the derive fields to be more consistent:

    Debug, Default, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash

Hash trait won't be impl in this PR due to rust std design.
If we need hash trait for f64 related structs in the future,
we should consider wrap f64 into a new type.

see: ratatui-org#307
@joshka joshka enabled auto-merge August 11, 2023 02:13
@joshka joshka added this pull request to the merge queue Aug 11, 2023
Merged via the queue into ratatui-org:main with commit 8c4a2e0 Aug 11, 2023
27 checks passed
@joshka joshka added this to the v0.23.0 milestone Aug 21, 2023
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