Skip to content

Conversation

j-g00da
Copy link
Member

@j-g00da j-g00da commented May 10, 2025

Since a lot of float methods are not included in core, I decided to make a simple polyfills crate, so we don't have to conditionally compile every place where these methods are used.

This adds a dependency, that does nothing if we use std. We can either:

  1. Leave as it is, with additional dependency.
  2. If we don't want to add a dependendy by default, we can introduce a std-polyfills (or no-std) feature flag (additional to std). I did something like that here: fix(no_std): final patches needed for no_std #1833.
    This has one drawback - we have to use either std or std-polyfills feature flag or the library won't compile.
    To make it easier on the user, we can add a check in lib.rs that emitts compiler error "either std or std-polyfills feature flag must be enabled" in such cases.
  3. I can try to make the float-polyfills crate lighter, not depending on libm (or gate libm behind feature flag).

Let me know what you think and if you have some other ideas for this.

Related: rust-lang/rust#137578

@j-g00da j-g00da requested a review from a team as a code owner May 10, 2025 15:49
@j-g00da j-g00da mentioned this pull request May 10, 2025
30 tasks
Copy link

codecov bot commented May 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.1%. Comparing base (54bb651) to head (bbb7915).
Report is 1 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1840   +/-   ##
=====================================
  Coverage   93.1%   93.1%           
=====================================
  Files         80      80           
  Lines      17693   17696    +3     
=====================================
+ Hits       16478   16481    +3     
  Misses      1215    1215           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@j-g00da
Copy link
Member Author

j-g00da commented May 11, 2025

Added these directly to core and widgets instead of adding a dependency as discussed on discord.

The uncovered lines are not important, it would be hard to test the polyfills trait implementation directly as std shadows it (and we need it to test against something), so I put the implementations in standalone functions and test these instead.

j-g00da and others added 2 commits May 11, 2025 12:53
Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>
@j-g00da j-g00da mentioned this pull request May 11, 2025
@j-g00da j-g00da requested a review from orhun May 11, 2025 12:00
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. It's possible that a few of these methods will no longer be necessary after some future work on the circle algorithms to avoid trig functions, but that can happen at a later date. For now this works to get over the no_std line.

@joshka joshka merged commit 00da8c6 into ratatui:main May 13, 2025
32 checks passed
joshka pushed a commit to MatrixFrog/ratatui that referenced this pull request May 15, 2025
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.

3 participants