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

Support input of hex, octal, and binary numbers #339

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

triallax
Copy link
Contributor

@triallax triallax commented Sep 20, 2022

Fixes #245.

I'm really, really ashamed of the terrible way I implemented this, but I honestly could not think of a way to do it that wouldn't be nearly as bad. Suggestions for alternate implementations or ways to improve the existing one are very welcome.

TODO:

  • Write tests

@triallax triallax force-pushed the support-n-base-digits branch from 504f6d3 to b3d6a2d Compare September 20, 2022 21:44
Comment on lines 125 to 129
numberPrefix /\ digit' /\ expSymbol ←
option ("" /\ digit /\ "e") $ char '0'
*> ((char 'x' <|> char 'X') $> ("0x" /\ hexDigit /\ "p")
<|> (char 'o' <|> char 'O') $> ("0o" /\ octDigit /\ "p")
<|> (char 'b' <|> char 'B') $> ("0b" /\ (oneOf ['0', '1'] <?> "binary digit") /\ "p"))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kill me, what have I done

@triallax triallax force-pushed the support-n-base-digits branch from b3d6a2d to 2ce4688 Compare September 20, 2022 21:47
@sharkdp
Copy link
Owner

sharkdp commented Oct 3, 2022

I'm really, really ashamed of the terrible way I implemented this, but I honestly could not think of a way to do it that wouldn't be nearly as bad.

I think it's perfectly fine as long as we add enough tests to make sure that everything is alright. Having a good test coverage also allows us to refactor with confidence later, if needed.

@triallax
Copy link
Contributor Author

triallax commented Oct 4, 2022

I think it's perfectly fine as long as we add enough tests to make sure that everything is alright. Having a good test coverage also allows us to refactor with confidence later, if needed.

Good point, I will add comprehensive tests.

@triallax triallax force-pushed the support-n-base-digits branch from 2ce4688 to 9b83aa0 Compare October 7, 2022 21:44
test/Main.purs Outdated Show resolved Hide resolved
@triallax triallax force-pushed the support-n-base-digits branch 2 times, most recently from 1ddd7d6 to 8f73b02 Compare October 11, 2022 22:36
docs/features.md Outdated Show resolved Hide resolved
Comment on lines +188 to +195
shouldFail "0xG"
shouldFail "0oG"
shouldFail "0oA"
shouldFail "0b2"
shouldFail "0be"
shouldFail "0x1p+"
shouldFail "0x1p-"
shouldFail "0x5.."
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍

Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the minor docu thing, this looks great. Thank you

@triallax triallax force-pushed the support-n-base-digits branch from 8f73b02 to 6753533 Compare October 13, 2022 20:42
@triallax triallax marked this pull request as ready for review October 13, 2022 20:44
@triallax triallax merged commit 7544300 into sharkdp:master Oct 13, 2022
@triallax triallax deleted the support-n-base-digits branch October 13, 2022 20:45
@triallax triallax changed the title Support hex, octal, and binary numbers Support input of hex, octal, and binary numbers Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support hexadecimal inputs
2 participants