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

Implement postfix parsing #953

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Implement postfix parsing #953

wants to merge 2 commits into from

Conversation

SanderSpies
Copy link
Contributor

@SanderSpies SanderSpies commented Jan 12, 2017

#637

Work in progress.

Changes:

50Cats

to

Cats 50.

Printing still needs to be done to finish this.

Copy link
Contributor

@yunxing yunxing left a comment

Choose a reason for hiding this comment

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

Looks good. Waiting for the test and printer change.

| postfix_chars
{
let l = Lexing.lexeme lexbuf in
POSTFIX (remove_numbers l, int_of_string (remove_chars l))
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: instead of using two functions, is it possible to just do one pass and return both number part and string part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

@chenglou
Copy link
Member

Thanks for the change! Btw #637 is pretty early. Let's see if people still want postfix...

@SanderSpies
Copy link
Contributor Author

Added printer support also and tests.

@SanderSpies
Copy link
Contributor Author

@chenglou This PR will be waiting patiently

@cullophid
Copy link

:+1

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.

None yet

5 participants