Prevent adjacent unary operators from merging #328
Conversation
facebook-github-bot
added
the
CLA Signed
|
@strulovich has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| | +-a | ||
| | -+a |
There was a problem hiding this comment.
should this be + -a and - +a? same below for +--a
There was a problem hiding this comment.
The intent was to test that the extra space is only added when it when two operators would merge.
We could also add a space here, but I'm not sure it makes sense for other combos.
There was a problem hiding this comment.
I probably misunderstand - do we want
var x = 5 + -a
or
var x = 5 +-a
?
or did I totally misunderstand?
There was a problem hiding this comment.
It's for unary operators, so the example would be:
var a = 1
println(+-a) // -1
I agree no human would ever write any of these examples, but a code generators write at least some of them.
| visit(baseExpression) | ||
| if (baseExpression is KtPostfixExpression && | ||
| baseExpression.operationReference.text.last() == operator.first()) { | ||
| builder.space() |
There was a problem hiding this comment.
is this exercised in any test?
There was a problem hiding this comment.
Not currently. The only example I can think of would be a!! !!, but a!!!! has the same semantics. Do you have a preference on that case?
| | +-a | ||
| | -+a |
|
|
||
| builder.token(operator) | ||
| if (baseExpression is KtPrefixExpression && | ||
| operator.last() == baseExpression.operationReference.text.first()) { |
There was a problem hiding this comment.
I think this relies on the fact that there are no operators with a mixture of characters, right?
I'm wondering if we should always put a space instead of guessing?
There was a problem hiding this comment.
That might be better? I like not guessing.
How do you feel about these cases:
a--!!
!--a
There was a problem hiding this comment.
I hate Kotlin :)
let's add these test-cases?
|
Otherwise looks good, thanks! |
|
@cgrushko has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@cgrushko has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
No description provided.