FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix `sprinf` with explicit zero precision and zero value by Earlopain · Pull Request #8769 · jruby/jruby · GitHub

/ jruby Public

Fix sprinf with explicit zero precision and zero value - #8769

Merged
enebo merged 1 commit into
jruby:masterfrom
Earlopain:d-format-precision-zero
Apr 16, 2025
Merged

Fix sprinf with explicit zero precision and zero value#8769
enebo merged 1 commit into
jruby:masterfrom
Earlopain:d-format-precision-zero

Conversation

Copy link
Copy Markdown
Contributor

In that case, nothing is written. It's documented: https://docs.ruby-lang.org/en/3.4/format_specifications_rdoc.html#:~:text=if%20the%20precision%20is%200%20and%20the%20value%20is%200%2C%20nothing%20is%20written

NOTE: I found this code quite hard to follow and there was at least one other place I contemplated implementing a fix. Not sure if what I chose is in fact the best place for a fix.

enebo added this to the JRuby 10.0.1.0 milestone Apr 15, 2025

enebo commented Apr 16, 2025

Copy link
Copy Markdown
Member

I have an ancient branch (#6913) which rewrites much of this but I got stuck on dtoa impl. We know this is horrifically messy and your fix looks fine even if it "maybe" could be somewhere else. As it stands this fixes the issue with new specs. LGTM. Also you may be passing some more tests in test/mri/ruby/test_sprintf_comb.rb (another fun piece of code).

enebo merged commit c3923e8 into jruby:master Apr 16, 2025
Earlopain added a commit to Earlopain/rubocop that referenced this pull request Aug 12, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

2 participants


Back | FazBrowse Home | New Git URL