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

#181 %.f fails with unexpected placeholder #188

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

#181 %.f fails with unexpected placeholder #188

wants to merge 1 commit into from

Conversation

ashwalk33r
Copy link

fix #181

change regex matching precision in order to reflect C documentation:
"if the period is specified without an explicit value for precision, 0 is assumed"

@alexei
Copy link
Owner

alexei commented Apr 3, 2020

Please provide unit tests

@ashwalk33r
Copy link
Author

ashwalk33r commented Apr 3, 2020

To be honest I do not know sprintf - what types of input are good to include in test? Can you guide me a little?

Basicly I change how ".precision" is interpreted - from digit(s) to digit(s) or 0 - what would be enough coverage in unit tests for that?

Idea 1
take existing precision tests (there is few) and change their precision to nothing and compare with .0

Idea 2
http://www.cplusplus.com/reference/cstdio/printf/
cover every case described in ".precision | description" section

Idea 3
Any place already containing tests that you know about (can be in diff language) that we can inspire from?

…if the period is specified without an explicit value for precision, 0 is assumed
@ashwalk33r
Copy link
Author

Best tests I could think of added.

@ricksterhd123
Copy link

Bump

@ashwalk33r
Copy link
Author

bump?

@ricksterhd123
Copy link

ricksterhd123 commented Nov 2, 2020

bump?

Can't remember why but I think this PR fixed some issue on another repo. Test still not good enough? It's been months.

@ashwalk33r
Copy link
Author

alexei doesnt care apperantly

@d0sboots
Copy link

d0sboots commented Jul 5, 2024

Can't remember why but I think this PR fixed some issue on another repo. Test still not good enough? It's been months.

The other repo/issue is fengari-lua/fengari#147 (still blocked on this, 5 years later). Just ran into this one myself. XD

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.

%.f fails with unexpected placeholder
4 participants