Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Updated ratatui to avoid mismatch (fixes #35) #36

Merged
merged 5 commits into from
May 23, 2024
Merged

Updated ratatui to avoid mismatch (fixes #35) #36

merged 5 commits into from
May 23, 2024

Conversation

tolik518
Copy link
Contributor

@tolik518 tolik518 commented May 6, 2024

This pr should fix #35.

The compile error in #35 is thrown due to a ratatui version mismatch. Some code had to be adjusted for the new version, but these were only minor changes

Also Im think its good to keep the libraries up to date :)

@tolik518
Copy link
Contributor Author

tolik518 commented May 6, 2024

Additionally, I had to swap the multibyte checkmark icon with a single-byte checkmark icon since it was breaking tests. Also, it could break the UI since it's rendered differently on different systems

Edit: even if the multibyte X was not breaking tests, it still might break the UI, so I swapped it also

@tolik518 tolik518 changed the title #35 updated ratatui to avoid mismatch Updated ratatui to avoid mismatch (fixes #35) May 6, 2024
@deepu105
Copy link
Contributor

Thank you. I'll check and merge

@coveralls
Copy link

coveralls commented May 20, 2024

Pull Request Test Coverage Report for Build 9162133625

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 7 of 8 (87.5%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.3%) to 67.226%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/ui/decoder.rs 5 6 83.33%
Files with Coverage Reduction New Missed Lines %
src/app/mod.rs 1 65.33%
Totals Coverage Status
Change from base Build 7646784226: 0.3%
Covered Lines: 1513
Relevant Lines: 1920

💛 - Coveralls

@tolik518
Copy link
Contributor Author

tolik518 commented May 20, 2024

@deepu105 I'm not sure why clippy is failing as I haven't touched AppResource

Edit: Since clippy is failing not as a result of my changes, I added an attribute which makes clippy skip that one particular trait. It looks like AppResource should be ultimately removed though.
Could you trigger the pipeline once again?

@deepu105
Copy link
Contributor

Thank you @tolik518 , Any reason you changed the icons for tick and cross? now they are not that obvious

@tolik518
Copy link
Contributor Author

tolik518 commented May 23, 2024

Hey @deepu105, yes, they were multi byte characters, which means that they get rendered differently depending on which font, shell and or terminal you use.
Futhermore a test was failing for me with the old icons which I could have fixed with some hacks, but that would make the tests more obscure and I don't think that would have been good practice.

Edit: I guess the tests weren't failing before that because ratatui might have changed some behavior for multi byte characters?

@deepu105
Copy link
Contributor

Ya seems like something changed at ratatui. I'll keep the ones you proposed for now

@deepu105 deepu105 merged commit 3ad2447 into jwt-rs:main May 23, 2024
5 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo install jwt-ui fails
3 participants