-
Notifications
You must be signed in to change notification settings - Fork 591
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
Rename scope punctuation.definition.numeric.base → constant.numeric.integer.base #2460
Rename scope punctuation.definition.numeric.base → constant.numeric.integer.base #2460
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting this rolling!
There are a number of places where constant.numeric.integer
is being used for a number scoped with constant.numeric.float
, which is incorrect. However, the more pressing issue to solve is the approach of the scope stacking and specialization. Currently:
- All of these rules result in stacking:
constant.numeric.integer.hexadecimal.cs constant.numeric.integer.base.cs
. There aren't nested numbers here, so the scopes shouldn't "stack". Previously thepunctuation
was stacked on top of aconstant.numeric
since it was a different scope. - If we simply remove the stacking, we will end up with one of two different scope styles, neither of which are particularly useful to color schemes:
- The
base
is a specialization of the main scope for the number. For example with the first pattern in C#, the0x
would beconstant.numeric.integer.hexadecimal.base.cs
and the rest would beconstant.numeric.integer.hexadecimal.cs
. This allows someone to target hexadecimal integer easily, but not be able to target all bases easily. - The
base
is not a specialization of the main number, but just ofconstant.numeric
orconstant.numeric.integer|float
. This would result in the0x
gettingconstant.numeric.integer.base.cs
and the rest gettingconstant.numeric.integer.hexadecimal.cs
. This would make it pretty trivial to target all bases (you need two rules, one for integers and one for floats), but you can't target an entire hexadecimal number at all.
- The
Pull request #2229 proposed adding some meta.
scopes to the constant.numeric
scopes. However, I feel like they would probably be better put on top, resulting in something like constant.numeric.integer.hexadecimal.cs meta.number.base
and the rest getting constant.numeric.integer.hexadecimal.cs
. This would make it possible to target constant.numeric meta.number.base
to target bases and constant.numeric.integer.hexadecimal
to target the entire scope. It is sort of unprecedented to stack meta.
scopes on top, but I think it provides a way to make it practical for color scheme authors to target what they want.
Part of @deathaxe's proposal from #2229 was:
meta
number
sign
base
value
mantissa
exponent
type
unit
I think that this may be a little too complex, partially from the implementation perspective, but also so nuanced that I'm not really sure color scheme authors will really ever use most of them. I do think that meta.number.base
should probably replace punctuation.definition.numeric.base
and meta.number.type
should probably replace storage.type.numeric
.
One benefit of the proposed stacking of the two meta.
scope on top is that we should be able to implement it pretty trivially with search and replaced throughout the syntaxes and tests. There shouldn't really need to be a shuffling of captures and whatnot.
In addition to feedback from @deathaxe, I'd love to get input from @FichteFoll, @jwortmann and @michaelblyons since they were involved with the discussion in #2135 related to numeric scoping. From a practical perspective, I'd love to try and keep the discussion focused on a mostly correct, but highly practical implementation. I'd prefer to not get too far into the weeds about exact semantics of number components, partially since I don't think that will have much real-world benefit, but also because we need to come up with guidelines that work across all syntaxes.
@wbond I figured the scope naming was going to be an issue here, so I just did a global replace to get the ball rolling here. Personally I'm in favor of the proposed meta scopes approach. |
Adopting the The result would be:
I'd propably try to keep the The basic idea behind |
I'm mildly in favor of the In no particular order:
|
No, it doesn't mess.
That's exactly the
Go used to highlighte the
That's barely possible without pushing into dedicated contexts for each digit as sregex doesn't support repetitions. You never know how many throwaway characters may be present. Thats just too much of everything - implementation and highlighting. |
I wonder if a lookahead match could check it is a valid integer (ie repetition shouldn't be a problem, nothing is being captured/scoped) , and push into a context which would match all digits and give them a |
I would agree to basically everything @wbond wrote above, i.e. to use
I don't agree with that, because with "type" I would always mean the data type here, i.e. how the number is stored internally. This is denoted by the For the exponent or the Regarding |
One thing I don't think has been addressed yet is the |
I agree that the scope for the decimal separator dot should contain |
I'm not sure I understand what you are referring to @BenjaminSchaaf and @jwortmann. You can definitely target the decimal point, at least that is what I am seeing in #2462. It is part of Generally we don't want people using Another observation is that |
You're right, I just saw the shift of However, I'm still unsure about its |
Those are |
... which uses the exact same color in Mariana, Sixteen and Celeste (#2229 (comment)). |
That's fine, use a different color scheme? I think we are getting far afield of the scoping of numbers by talking about color schemes. |
I indeed use my own color scheme, I was just expressing my personal opinion about the visual style from giving the decimal dot a It's the same as with the current prefix |
I was looking at the existing Ruby syntax, and it uses that, so I don't think we are breaking new ground with #2462.
The The |
#2462 is really helpful in this dicussion. Conceptually I believe scopes should generally modify their latest part for differences in nuances where possible, such as denoting the value or suffix part of a number token. Since the entire token is If
That would be a breaking change of course, but it allows the meta scope to be below the constant scope and still only modify the last scope within a number construct. (I also like simply using "suffix" to catch the discussions of what a suffix would actually be. People may want to be more specific and add another sub-scope, if they really want to.) |
Using I personally wouldn't be worried about maintaining Hence I'd just replace it by a general Besides the fact it is some kind of backward compatibility issue, I honestly thought about @FichteFoll's proposal when I reworked the Ruby and JavaScript syntaxes yesterday. I can absolutely follow his arguments and kind of like it. Why? From a color scheme perspective the detailed type of a number may not be too important. I guess only few will ever colorize number types differently. Improving readability of hexadecimal floating point numbers like |
I like the suggestion from @FichteFoll as well, but I guess he meant to include the number kind into the I think this approach is easy to implement and useful for color schemes. I could prepare a PR for Matlab as illustration. |
I've put in a PR for C# that takes the same approach. |
I like how these two turned out. The contexts and scope definitions are easy to read and reason about and contain the desired precision while still allowing color schemes to target whatever they want. |
Applied it to Ruby as well. With a look at the other PRs it feels like the correct solution. |
Yes, the
Alright, so the summary is that we either need to keep |
Here's the path forward:
It looks like we've got PRs open for:
We still need to address:
If you are planning to do a PR for one of these, please comment here before starting so we don't duplicate work. |
Will update JavaScript to the latest scheme. Already did that in Java. Could create a dedicated PR as there are still some things I want to work on in my rewrite branch. So can't tell when it's done. May have a look at python and yaml, too. |
It looks like #2308 is probably the right place to address C/C++, right? |
We could abuse it for meta.numbers, yes or just merge it as preparation for a rework. A more general question about C/C++ is also whether we want to push into contexts to highlight single invalid characters or whether just find some more permissive/relexed rules. |
Alright, I'll see if I can provide guidance to merge that, then we'll handle the meta number stuff separately. |
I can do a PR for Lua. |
YAML may take a little longer. May need to split the already huge pattern a bit for better readability. |
I'll most likely be working on these today. If anyone is keen to do one, give a shout. I'll update the list as I go. |
Continuing with Perl |
Working on D |
Working on C/C++/Objective-C |
Working on Go |
Working on Haskell |
I'm going to look at OCaml, Clojure and Scala now |
Scratch that, I ended up spending my time on the TypeScript -> TSX transition instead. Anyone else is free to work on Clojure, OCaml and Scala. |
Working on Clojure |
Working on OCaml |
Working on Scala |
All of the syntaxes that were part of this PR have now been updated with the Thanks for all of the work by everyone on this. It was one of the most significant remaining steps in getting the default syntaxes ready for ST4. |
See sublimehq/Packages#2460 for the discussion.
0b
and related should not be punctuation. There's currently a workaround in the default colorschemes for this but this is a proper fix.