[YAML] Replace entity.name.tag with meta.mapping.key

[FichteFoll]

I've been wanting to do this for a while, as entity.name.tag was really just a hack for a lack of an appropriate scope name when I made this syntax. Now that JSON, JavaScript and Python (and probably other syntaxes) successfully use meta.mapping.key, YAML should not be exempted.

---

(via #2564 (review))

Without addressing sublimehq/sublime_text#3590, the UX of default color schemes with regards to highlighting keys differently from values will be worse but in line with JSON and the other languages using meta.mapping.key as the primary differntiator between keys and values.

The only thing needed for Mariana is something like:

{
	"name": "Mapping Key Names",
	"scope": "meta.mapping.key string",
	"foreground": "var(red2)"
}

Monokai should be fixed as well to maintain the highlighting users are used to.

[deathaxe]

meta.mapping and meta.sequence ... nice.

[FichteFoll]

Thanks for the suggestions. I think I addressed all your comments, please re-review.

[deathaxe]

LGTM

[michaelblyons]

If this goes through, Mariana.sublime-color-scheme needs updating. Maybe others?

[deathaxe]

But seems more consistent with a look at how integers etc are scoped in keys.

But found a situation of missing meta.mapping.key ...

The : is scoped using meta.mapping while key and value receive meta.property only.

[jwortmann]

If this goes through, Mariana.sublime-color-scheme needs updating. Maybe others?

👉 sublimehq/sublime_text#3590

[FichteFoll]

The : is scoped using meta.mapping while key and value receive meta.property only.

The key is also scoped as a key for me, but the property isn't. I'll add that to the look-ahead for correctness, but values currently aren't scoped in block collections at all, because matching is too inaccurate for those. We can add that later, imo.

[FichteFoll]

Turns out this isn't as simple as I was hoping to and it does make the syntax unnecessarily complex, imo. What should happen is rewriting the key and value pair detection using branching, but I won't get to that in the near future, so this quick(er) fix should do for now.

[wbond]

Beyond the default color schemes, this is going to need a forward fill for color schemes in general. Moving from entity.name.tag to string.unquoted will leave the syntax pretty bare in most color schemes.

[deathaxe]

I wonder if we can find some kind of scope for such mapping keys in general. We need it in python mappings, json and even CSS might be a candidate to apply the meta.mapping to properties.

Basically, even though it complies with JSON/YAML specs, I find it a bit weird to scope keys string. A string feels more like a value or argument. In case of YAML, which is also used in PackageDev it makes navigation via goto definition harder/impossible as string is always excluded from such functions.

[wbond]

"Symbols" in Ruby use constant.other. What about something like constant.other.key for unquoted key values?

[deathaxe]

It definitly might be a good scope in case a key is used to access an item, like dict[key], but I am not quite sure how well it plays in CSS for instance. Nearly all key like tokens (html-tag names, attribute names, property names, ...) use entity.other or entity.name there, at the moment.

If we think of meta.mapping to be a form of structured data like XML and JSON, YAML as an alternative representation of such, something like entity.name.tag makes sense. At least I understand why it was choosen in the past.

With the intention of #1861 in mind, too, I wonder if such a key should stay within the entity. scope?

Not sure if it needs to apply to unquoted keys only. In Erlang for instance all identifiers may be quoted, without them denoting strings. The same applies to SCL/ST (structured text) used in automation systems.

One point of this PR is to avoid too much scopes being stacked. Hence I guess entity was removed in favor of meta.mapping.key, but finally scoping keys as string is the same as scoping them boolean which has been fixed as well. TCL also chooses to scope strings semantically as nearly everything is one, etc.

[wbond]

So perhaps something like entity.other.key and skipping the string.unquoted?

I could potentially see using entity.other.keyfor string keys in JSON, and performing a forward fill of a color scheme rule to populate it fromstring`. That would preserve existing colors, but also give color schemes the ability to provide special colors for all mapping keys, wether they be unquoted, quoted, etc. The only thing we probably wouldn't want to scope would be something like a number as a key, right?

[deathaxe]

I have a good feeling about entity.other.key. I would remove string as well, then, yes. The meta.mapping.key is then used to denote expression like keys as the property thing discussed here.

The point about meta.mapping string in json is, it already "broke" or removed highlighting the same way as we discuss it here for older color schemes due to removing the scope used for keys before. I haven't investigated but maybe entity.other would even improve or restore the original behavior for them even without a forward fill?

[jwortmann]

@wbond:

So perhaps something like entity.other.key and skipping the string.unquoted?

I'm not sure about entity.other.key. Not that I'm per se against it, and I can see that it's kind of unfortunate to have string for both keys and values, which is why I asked in the linked issue above to distinguish them via the meta.mapping.key scope in the default color schemes. But the only entity.other.* scopes in use seem to be entity.other.attribute-name and entity.other.inherited-class, and entity.other is not beyond the recommended minimal scope coverage for color schemes. So how exactly would such a forward fill from string exactly work? Let's assume entity.other.key is not defined, then take the color for string instead => that would mean that color schemes which have adopted and assigned a different color for meta.mapping.key string in the meanwhile (it is in use in the lasted ST3 release at least for JSON and quoted keys in Python) would "break". Or alternatively, if the color from meta.mapping.key string is taken instead => this would not be effective as most color schemes probably don't have a rule for it. It might work if for both entity.other.key and meta.mapping.key string is tested and the correct color is taken accordingly, but that makes things quite complicated.

And besides JSON, which is a strict subset of YAML, such a change would also require changing it again in JavaScript, Python, Lua and possibly more if we aim for consistent scopes between languages.

Furthermore, as you already mentioned, it's unclear to me whether entity.other.key should only apply to string keys or also to other kind of keys. For example, Python or Julia allow almost arbitrary things as keys, e.g. numbers or custom types. Keys that are numbers are currently scoped as constant.numeric in Python, should this be changed to entity.other.key as well then? If not, what makes string keys special from other types of keys, that they "deserve" entity.other.key?

@deathaxe:

I am not quite sure how well it plays in CSS for instance. Nearly all key like tokens (html-tag names, attribute names, property names, ...) use entity.other or entity.name there, at the moment

I think these are only CSS selectors and I would say they are somewhat different from key-value mappings. The properties actually use support.type.property-name, and this should be fine, because only predefined properties with known names are allowed (afaik?). So in my opinion it's not necessary to change the CSS scopes, especially since they are quite unique and there are probably a lot of color schemes which specifically target them.

The point about meta.mapping string in json is, it already "broke" or removed highlighting the same way as we discuss it here for older color schemes due to removing the scope used for keys before

JSON used string for keys even before, I believe the only difference is a rule for "meta.structure.dictionary.json string.quoted.double.json" in Monokai, which doesn't apply anymore. Therefore I can only reference sublimehq/sublime_text#3590 again as a pretty easy fix. As explained above, I guess entity.other (without a forward fill) would not apply any color at all in most color schemes, so it would fall back to the default foreground color. Not sure whether that's an improvement or not.

[deathaxe]

The main issue I have with string is it being treated special by many functions. It's a major reason for goto definition not to work properly in PackageDev's sublime-syntax syntax. Each key (context name) is a string and thus excluded by ST's symbol.py

Of course, we can define a special syntax for that which doesn't use strings for context names. But that's basically YAML, so it makes sense to use the same base scopes. There are other use cases where users want goto defintion functionality for enumeration keys in C/C++. How is that so much different to structured data?

Otherwise the meta.mapping [string|int|constant| ...] approach is pretty much ok.

[FichteFoll]

but finally scoping keys as string is the same as scoping them boolean which has been fixed as well.

No, boolean keys true and false are still scoped as booleans. What was removed were the matches for boolean aliases in keys, i.e. y: 12.

---

In general, I believe we have the following ways forward:

1. Scope every key as mapping.key and then scope the key's token as it would be when written as a value, i.e. use string for a string and use constant.numeric for numbers. Python example: {"string": "hi", 2: 3}. Syntaxes with implictly unquoted string keys, such as JavaScript, would then use string.unquoted so they wouldn't be confused with an identifier and seeing how they are equivalent to strings. Ruby's symbols would still get constant.other.symbol.

2. Use entity.other.key for scalar keys, such as strings (quoted or unquoted) or integers. For quoted strings, we wouldn't want to include punctuation.

3. Do both.

The following evaluation is from the perspective of "what if we started from zero". All solutions would require a forward-fill for compatibility with old color schemes anyway.

Advantages:

1. Low-level approach that allows fine-grained control for color schemes or plugins.

2. It becomes clear what token represents a key. Color schemes can simply target the key scope and be done with it.

3. Both of the above.

Disadvantages:

1. meta.mapping.key string also matches strings in complex keys, e.g. {["list", "key"]: "value"} (Python). Context management of clearing the non-key meta.mapping scope is sometimes not so straight-forward. string is special-cased for some ST features like goto definition.

2. Can't re-use expression-style contexts for matching keys, as entity.other.key needs to be applied on top. What is entity.other supposed to mean in the long run? What do we do for non-static values such as interpolated strings or identifiers used as keys.

3. Both of the above. Also more scope stacking.

I should mention that if we were to go for 2, I would question the usefulness of meta.mapping.{key,value}, as the primary goal of that has been to distinguish between key and value expressions. We may be able to remove this scoping, then.

[jwortmann]

... in complex keys, e.g. {["list", "key"]: "value"} (Python)

This expression is illegal in Python, only immutable types can be keys, but lists cannot.

Otherwise I agree, if we use only entity.other.key instead of the actual type for keys, then meta.mapping.{key|value} has no more use and could be removed.

[deathaxe]

Maybe ST can just ignore its string exclusion in meta.mapping.key to enable goto definition and everything is fine.

[michaelblyons]

Maybe ST can just ignore its string exclusion in meta.mapping.key to enable goto definition and everything is fine.

I like this for an additional reason: YAML keys can be explicitly quoted strings, too:

foo:
  - bar
  - baz
'bim': true

Unfortunately, they're not scoped as keys at present.

This expression is illegal in Python, only immutable types can be keys, but lists cannot.

Because I'm silly and pedantic sometimes: sure, but that list as a tuple can be a key. {("list", "key"): "value"} If you want to be even sillier, you can use namedtuples and pretend your key is a dict.

[jwortmann]

Because I'm silly and pedantic sometimes: sure, but that list as a tuple can be a key. {("list", "key"): "value"} If you want to be even sillier, you can use namedtuples and pretend your key is a dict.

True, it indeed works with tuples. But I guess examples like this aren't used very often in practice, so I wonder how relevant it is for this discussion. And actually, I would see this more as an advantage for meta.mapping.key than a disadvantage, because the meta scope can span the whole tuple (since the key does consist of the whole tuple here), while the parentheses and comma can get punctuation and the strings get string.quoted.double as expected.
In case of using entity.other.key instead, would you apply this scope to all of the characters, including the punctuation and spaces? That might make it a bit more visible that the whole tuple-structure is the key, but at same time it would probably look quite unusual if all these characters have the same highlighting color then.

But I'm not sure which way is simpler or more robust from the implementation side and how exactly it is implemented right now in Python.

[FichteFoll]

In case of using entity.other.key instead, would you apply this scope to all of the characters, including the punctuation and spaces?

In my proposal, I would only apply it to scalar elements, i.e. strings and numbers and without punctuation, since this would be more of a semantic scope name.

[jwortmann]

In my proposal, I would only apply it to scalar elements, i.e. strings and numbers and without punctuation, since this would be more of a semantic scope name.

Ok, but is it really useful to drop the punctuation, i.e. the string quotes from entity.other.key even for scalar elements? While it wouldn't make a semantic difference in JavaScript (where unquoted keys implicitly get converted to a string), in Python the keys "42" and 42 aren't the same. For item access like mydict["42"] you also can't just omit the quotes in Python, if you want to select the quoted key. So I would expect the entity.other.key scope to match the same behaviour. And in case of Goto functionality or something like a symbol list for mapping keys, which might use these scopes, I wouldn't expect two unquoted identical entries 42 if there exist both a quoted and an unqouted key, which are actually different. Maybe this decision should be syntax dependent, based on whether the language does an implicit string conversion or not?
Assuming no entity.other.key for the punctuation, and if there is also no more string scope for the key anymore, should the quotes still get punctuation.definition.string then? Most color schemes, such as Monokai, probably don't target punctuation.definition alone, so the quotes wouldn't get any color at all.

Anyways, I took some time and made a few experiments in Monokai and Mariana for the resulting highlighting colors, assuming a rule for meta.mapping.key string or entity.other.key, respectively, will be added to the color schemes. That should probably be equivalent to the suggested "forward fill", even though I still don't know how exactly or with which scopes this forward fill should work (or is it perhaps meant to apply only to YAML?). Note that Mariana already has special rules for entity.name.tag.yaml and for source.yaml string.unquoted. So for Mariana I used the color from entity.name.tag.yaml for these extra rules, while for Monokai I used orange as an example, because the color for entity.name.tag is the same as for keyword in that scheme, which would look a bit confusing in Python code.

Current highlighting (as of latest ST3):

Alternative 1 (this PR): meta.mapping.key and actual type, with a corresponding color scheme rule for meta.mapping.key string:

Alternative 2: entity.other.key, without string, but only for scalar keys and excluding the punctuation as suggested. In Mariana the excluded quotes make no difference (assuming they still get punctuation.definition.string or a similar scope), because Mariana has a rule for punctuation.definition with the same color as for entity.name.tag.yaml. But for Monokai it looks kind of "incomplete" to me without any highlighting for the quotes:

I would still prefer the meta.mapping.key solution, which allows to color e.g. number keys differently from string keys.

[michaelblyons]

I currently support meta.mapping.key, although if I were to make a color scheme rule, I'd exclude the tuple coloring (meta.mapping.key - meta.mapping.key meta.group or something).

With DeathAxe's idea to drop the string exclusion for go-to refs, I think that covers the things I'm interested in.

[deathaxe]

Yeah, color schemes need to be tweaked anyway as JSON looks pretty plain as well.

Maybe someone should have a look into those merge conflicts. I am fine with this PR as well.

[deathaxe]

Situation won't change with this PR assuming id keeps string scopes being applied to keys. But as JSON also scopes keys string a solution including meta.mapping.key in Default/symbol.py might be needed anyway.

[deathaxe]

An attempt to fix quoted meta.mapping.key can be found at FichteForks#10

[FichteFoll]

Applied a similar solution to block keys as well while fixing some invalid explicit block key syntax.

https://github.com/yaml/yaml-editor with the c-libfyaml parser is very useful for verifying YAML syntax, by the way.

[jrappen]

Two things I noticed:

• still using pop: true instead of pop: 1, but you wanted to change that later in another PR
• the use or non-use of *.yaml-endings for scopes in tests is really random

[deathaxe]

LGTM! Maybe two tiny code style improvements...

[deathaxe]

the use or non-use of *.yaml-endings for scopes in tests is really random

That's a general issue in most syntaxes. Tests started quite simple when introduced and matured to capture more edge cases and details. I guess PRs with improved test cases are wellcome in general.

[deathaxe]

With another headsup for sublimehq/sublime_text#3590 to @sublimehq to provide a good UX with regards to highlighting JSON/YAML, I am fine with this PR.

Note: Not patching the builtin color schemes will cause YAML to look pretty plain with this PR as JSON already does for years now, unfortunatelly.

The only thing needed for Mariana is something like:

{
	"name": "Mapping Key Names",
	"scope": "meta.mapping.key string",
	"foreground": "var(red2)"
}

Monokai should be fixed as well to maintain the highlighting users are used to.

[jrappen]

@FichteFoll

• Maybe you could update the opening post for the commit message of the squash merge?
• Maybe mark this as addressing 3228?

[FichteFoll]

The contents of a squash merge commit are taken from all the squashed commits, afaik, so changing the OP should not matter here. I added @deathaxe's note regarding sublimehq/sublime_text#3590, however.

Since the scoping of key-value separators mentioned in 3228 wasn't changed in this PR, it doesn't address anything of it (outside of not changing/violating it).

[jrappen]

🤷🏼‍♂️ Not sure why you made the commit message of the merged commit loose all the details that were provided above.

[FichteFoll]

Same here 😒