feat(ufmt): enhance printf functionnalities by adding flags
Fixes
ufmt/printf- added flag parsing, such as
%.number,%#,%0, and others - cf 5d71b3a
- new features has been tested using unit tests (see tests)
- added flag parsing, such as
🛠 PR Checks Summary
All Automated Checks passed. ✅
Manual Checks (for Reviewers):
- [ ] IGNORE the bot requirements for this PR (force green CI check)
- [ ] The pull request description provides enough details
Read More
🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.
✅ Automated Checks (for Contributors):
🟢 Maintainers must be able to edit this pull request (more info) 🟢 Pending initial approval by a review team member, or review from tech-staff
☑️ Contributor Actions:
- Fix any issues flagged by automated checks.
- Follow the Contributor Checklist to ensure your PR is ready for review.
- Add new tests, or document why they are unnecessary.
- Provide clear examples/screenshots, if necessary.
- Update documentation, if required.
- Ensure no breaking changes, or include
BREAKING CHANGEnotes. - Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
- Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)
If
🟢 Condition met └── 🟢 And ├── 🟢 The base branch matches this pattern: ^master$ └── 🟢 The pull request was created from a fork (head branch repo: paulogarithm/gno)Then
🟢 Requirement satisfied └── 🟢 Maintainer can modify this pull requestPending initial approval by a review team member, or review from tech-staff
If
🟢 Condition met └── 🟢 And ├── 🟢 The base branch matches this pattern: ^master$ └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)Then
🟢 Requirement satisfied └── 🟢 If ├── 🟢 Condition │ └── 🟢 Or │ ├── 🟢 User notJoon already reviewed PR 4136 with state APPROVED │ ├── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request │ └── 🔴 This pull request is a draft └── 🟢 Then └── 🟢 And ├── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending) └── 🟢 At least 1 user(s) of the team tech-staff reviewed pull requestManual Checks
**IGNORE** the bot requirements for this PR (force green CI check)
If
🟢 Condition met └── 🟢 On every pull requestCan be checked by
- Any user with comment edit permission
The pull request description provides enough details
If
🟢 Condition met └── 🟢 And ├── 🟢 Not (🔴 Pull request author is a member of the team: core-contributors) └── 🟢 Not (🔴 Pull request author is user: dependabot[bot])Can be checked by
- team core-contributors
Can you split it into two?
@thehowl done
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:loudspeaker: Thoughts on this report? Let us know!
@notJoon i'm done fixing everything 👍 i just need your opinion on something (concerning backward pointer checking usefullness)
tell me if i need to add anything else !
@paulogarithm
Hmm... When using ufmt (especially, using Sprintf function), gas consumption increases causing "out of gas" errors and test failures. The other CI failure issues will likely be resolved with a branch update, but this one seems to require code modifications.
I'll check this problem today. If it's okay, may I modify this code together with you?
totally fine for me, @notJoon ! but maybe it's because of the maps, so when you'll test, can you try putting the maps in the scope like i did, even if it's more greedy on memory ?
can you try putting the maps in the scope like i did, even if it's more greedy on memory ?
I'm testing by directly checking the gas consumption with txtar. It seems rather than map usage, we also need to improve parts related to memory allocation within the ufmt package overall.
@paulogarithm I couldn't push directly to this PR #4174, so I created a separate PR. Please check it and feel free to cherry-pick if needed. thank you 👍
@paulogarithm did you have a chance to read guilhem's feedback?
I'm so sorry i was putting all my efforts on Flamegraphs & EVM, I did not made any updates, but I'm back !
- @thehowl yep i'm reading each reviews
- @gfanton i will add the new tests + improve the Sprintf doc + check your reviews
- @notJoon ill cherry pick mostly everything because from what i seen, it seems that everything improves ufmt in a better way !
alright so i added the tests from the real printf (the ones that @gfanton shared here) + i cherry picked (more like manually copy-pasted and put notJoon as co authored) the @notJoon 's code parts for optimization. the only things that are mostly left on the tests are the G flag
@gfanton @thehowl sorry for the late reply, tell me if everything lgty guys
See-also https://github.com/gnolang/gno/pull/4174#pullrequestreview-3115281780
Closing this issue; Pol won't be working on it as he is no longer on the student contributor programme and doesn't have time to continue working on this </3
@notJoon, I think you can pick it up and finish it :)