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

Fixed operator precedence #387

Merged

Conversation

TheGrandmother
Copy link
Contributor

Operator precedence is now normal.
Now equality checks bind stronger than logical operators.

Before one had to write 1 == 2 and 5 <= 2 as (1 == 2) and ( 5 <= 2 )

Relates to issue #385

@supercooldave
Copy link

supercooldave commented Apr 16, 2016

I'd like to see some more extensive testing, which will help define/understand what normal is. A single file test with lots of possibilities should do the trick.

Your tests could be simply showing that 1 == 2 and 5 <= 2 and (1 == 2) and ( 5 <= 2 ) are the same, for many such expressions.

@TheGrandmother
Copy link
Contributor Author

@supercooldave
Oki I'll fix. Just didn't think this was a big enough thing to test. But I'll do it.

@TheGrandmother
Copy link
Contributor Author

TheGrandmother commented Apr 17, 2016

Added tests.
But wait with merging until #380 is merged.

The tests uses the new printing to avoid having to change them once the new printing is merged.

@albertnetymk
Copy link
Contributor

#380 is merged, better to do a rebase to make CI happy.

@TheGrandmother
Copy link
Contributor Author

Fixed!

@albertnetymk
Copy link
Contributor

operator_precedence.enc breaks some coding convention, but it's very minor. I would merge it in one hour if no objections.

@supercooldave
Copy link

@albertnetymk This is not a very useful comment.

@albertnetymk
Copy link
Contributor

albertnetymk commented Apr 19, 2016

@supercooldave Because I didn't elaborate on what coding conventions are broken or what?

@supercooldave
Copy link

@albertnetymk Precisely. Your comment was akin to "Something is wrong – but I'm not going to tell you what it is."

@TheGrandmother
Copy link
Contributor Author

@albertnetymk
Is the coding standard defined somewhere?
I was made aware that we had one during the hackathon :p

@albertnetymk
Copy link
Contributor

There's no strict coding standard, but we have a convention not to exceeding 80 columns and not having trailing spaces. They are very minor. I would merge it as it is if you don't feel like fixing it.

@TheGrandmother
Copy link
Contributor Author

Fixed!

@EliasC
Copy link
Contributor

EliasC commented Apr 19, 2016

This looks reasonable! Merging before I go home today.

@albertnetymk albertnetymk merged commit d73c164 into parapluu:development Apr 19, 2016
@albertnetymk albertnetymk deleted the features/precedence-fix branch April 19, 2016 22:30
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.

5 participants