This repository was archived by the owner on Feb 13, 2025. It is now read-only.
Show parentheses when outputting expressions #2456
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Show parentheses when emitting expressions Problem ======= When showing evaluated expressions (e.g. in the computations column when testing an expression or an alert), the expressions are ambiguous (and misleading) because they include no parentheses, even where they are necessary. E.g. evaluate the following expression: ``` (1 + 2) / 3 ``` and the computations column will show: ``` 1 + 2 / 3 = 1 1 + 2 = 3 ``` Compare to evaluating the expression: ``` 1 + (2 / 3) ``` which will show: ``` 1 + 2 / 3 = 1.66667 2 / 3 = 0.66667 ``` Solution ======== The simplest fix is to wrap all binary expressions in parentheses. This _does_ emit redundant parentheses, but it's much simpler than figuring out when precedence rules allow the safe elision of parentheses. So for example this expression: ``` 1 + 2 + 3 + 4 ``` will be rendered: ``` (((1 + 2) + 3) + 4) ``` I think any extra verbosity here is worth paying for being explicit and unambiguous. An alternative solution would be to apply the parentheses in the _parent_ node - unconditionally if the parent node is a unary expression and the child is a binary expression, and conditionally if the parent node is a binary expression and the child is a binary expression: in this case parentheses are required on the left child node if it binds less tightly than the parent's operator, and are required on the right child node if it binds less or equally tightly than the parent's operator. This is far too painful to think about so I'm not going to.
Problem: Now that we _always_ wrap parentheses around binary operations we can produce expressions that are needlessly verbose. E.g. ``` (((1 + 2) + 3) + 4) + 5 ``` Solution: Eliminate all the redundant parens - i.e. those that won't change the parsed expression tree. The main place parentheses need to remain are: 1. when we want to apply a lower precedence binary operation (e.g. +) before a higher precedence operation (e.g. *), 2. if we want to apply a left-associative binary operator from the right instead (e.g. subtractions such as `3 - (2 - 1)` ), and 3. if we want to apply a binary operation before a unary operation (all binary operations in the expression language have lower precedence than all unary operations).
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Contributor
Author
|
I would still like to propose this. It solves a real problem. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Problem
When showing evaluated expressions (e.g. in the computations column when
testing an expression or an alert), the expressions are ambiguous (and
misleading) because they include no parentheses, even where they are
necessary.
E.g. evaluate the following expression:
and the computations column will show:
Compare to evaluating the expression:
which will show:
Solution
The simplest fix is to wrap all binary expressions in parentheses. This
does emit redundant parentheses, but it's much simpler than figuring
out when precedence rules allow the safe elision of parentheses. So for
example this expression:
will be rendered:
I think any extra verbosity here is worth paying for being explicit and
unambiguous.
An alternative solution would be to apply the parentheses in the
parent node - unconditionally if the parent node is a unary expression
and the child is a binary expression, and conditionally if the parent
node is a binary expression and the child is a binary expression: in
this case parentheses are required on the left child node if it binds
less tightly than the parent's operator, and are required on the right
child node if it binds less or equally tightly than the parent's
operator. I have now implemented this and added it to the PR.
Fixes #2455 - Feature request: show parentheses in formatted expressions
Type of change
From the following, please check the options that are relevant.
How has this been tested?
Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration
Checklist:
addedupdated tests that prove my fix is effective or that my feature works