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

Stop splitting common operators #275

Merged
merged 8 commits into from
Oct 11, 2024
Merged

Stop splitting common operators #275

merged 8 commits into from
Oct 11, 2024

Conversation

Jentsch
Copy link
Contributor

@Jentsch Jentsch commented Oct 9, 2024

This PR fixes the operators like ->, :+, =>, <- that are 'composed' from shorter operators like - and >. It also fixes operators like \/ where \ wasn't highlighted at all.

The change looks quite drastic, but the changes in the snaps look fine to me. I'd would appraciate a second pair of eye to check if an special operator is missing.

A change where I'm not sure if we want it:

   i -= 1
//  was:
//   ^ keyword.operator.arithmetic.scala
//    ^ keyword.operator.comparison.scala
//  now:
//   ^^ keyword.operator.scala <- 

One could argue, the - is realy the arithmetic operator here, as the
expressions i -= 1 most of the time means i = i - 1.

Btw. I don't think in i = 1 the = shouldn't be marked as a comparison but just as an operator.

@bishabosha
Copy link
Member

I think that because you are allowed to define def -= (foo: Int) ... that the interpretation is fine

@Jentsch
Copy link
Contributor Author

Jentsch commented Oct 9, 2024

I changed how operators are recognized and I'll update the PR text.

@Jentsch Jentsch marked this pull request as ready for review October 9, 2024 13:00
name: 'keyword.operator.arithmetic.scala'
},
{
match: '(=|\\<|>)',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I think we shouln't match =, as a single = doesn't compare but defines things.

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

Makes sense! You need to rebase on main to get this mergable though

Doesn't affect comparison, arithmetic nor logical.

In the match `:+` the `+` became redundant, because it's contained in
`opchar`.

Discovered while working on scala#191.
'Longer' operators like -> aren't an arithmetic operator
and than a comparison, but one operator.
With this the examples from
[the documentation](https://docs.scala-lang.org/tour/operators.html)
work.
Instead of dancing around around special operators
we can just enumerate them now. This fixes a lot of
edge cases.
@Jentsch
Copy link
Contributor Author

Jentsch commented Oct 11, 2024

Done

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

Thanks!

@tgodzik tgodzik merged commit f92d65a into scala:main Oct 11, 2024
2 checks passed
@Jentsch Jentsch deleted the fix-operators branch November 24, 2024 21:18
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.

3 participants