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.

Sign up for GitHub

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

Jump to bottom

[JSON] Meta scopes for mapping and sequence #862

Merged
merged 6 commits into from
Feb 8, 2019
Merged

[JSON] Meta scopes for mapping and sequence #862

merged 6 commits into from
Feb 8, 2019

Conversation

FichteFoll
Copy link
Collaborator

@FichteFoll FichteFoll commented Mar 29, 2017 •
edited
Loading

In direct response to #421.

Use meta.key and meta.value for key and value tokens respectively. In the process, generalize meta.sequence and meta.mapping (and "fix" their punctuation scopes).

Investigation on how this might affect third-party color schemes should likely be performed, for the latter change only, which I doubt had any special coloring applied.
The scopes changed in the first change have only been added in 5e24e2b (#816), which hasn't been part of any build yet.

FichteFoll added a commit to SublimeText/PackageDev that referenced this pull request Mar 29, 2017
@FichteFoll
Rename JSON meta scopes
fc39713 
Refer to sublimehq/Packages#421 and
sublimehq/Packages#862.
@wbond
Copy link
Member

wbond commented Mar 30, 2017

Why meta.mapping.json meta.key.json and not specialize to meta.mapping.key.json?

I can see the appeal of meta.value.json since it can be used in a mapping and sequence.

@wbond
Copy link
Member

wbond commented Mar 30, 2017

Actually, it doesn't appear that meta.value is being applied to sequence values.

So this is just changing the scope names, correct?

@FichteFoll
Copy link
Collaborator Author

I didn't chose meta.mapping.key because the token would (most likely?) already have the meta.mapping scope applied to them. Furthermore, it allows to target keys individually outside of a mapping, although I don't think this is ever going to happen, since key-value storages are pretty much always mappings.
Thus, it probably makes sense to have "key" and "value" under meta.mapping to better indicate their purpose on their own.

I don't exactly see a merit in a meta.sequence.value scope, since meta.sequence - punctuation.separator.sequence should cover this already, pretty much, should it ever be needed. I could add it anyway, of course.

@FichteFoll FichteFoll mentioned this pull request Apr 7, 2017
Open
@FichteFoll FichteFoll mentioned this pull request May 26, 2017
Merged
@FichteFoll FichteFoll changed the title [JSON] Meta scope [JSON] Meta scopes for mapping and sequence Aug 21, 2017
@FichteFoll FichteFoll mentioned this pull request Aug 21, 2017
Closed
@FichteFoll FichteFoll mentioned this pull request Sep 20, 2017
Merged
@FichteFoll
Copy link
Collaborator Author

Any update on this?

@wbond
Copy link
Member

wbond commented Nov 28, 2018

@FichteFoll mind looking into the conflicts on this? Since it only affects meta. scopes, it should be fine to merge into the current dev cycle.

@FichteFoll
[JSON] Move value context to the top
c279e33
@FichteFoll
[JSON] Use meta.key and meta.value scopes
6db72c9 
The older ones are significantly longer and don't exactly convey more
meaning than these shorter ones. This should help unify scopes for
key-value structures across languages.
@FichteFoll
[JSON] Use more general "sequence" and "mapping"
5f29a82 
The terms are more general across languages and carry sufficient
semantics.

Also use the proper `punctuation.section` scope for mapping and sequence
punctuation.
@FichteFoll
[JSON] meta.{key,value}-> meta.mapping.{key,value}
fc610c7
@FichteFoll
[JSON] Use prototype context
40b4dcb
@FichteFoll
[JSON] Don't stack meta.mapping and *.{key,value}
5643334 
Stacking prevents color schemes from highlighting inner keys in
different colors, since a selector like "meta.mapping meta.mapping.key"
would also match the first level, but "meta.mapping meta.mapping
meta.mapping meta.mapping.key" would match the second and not the third.

In the process, define and assign more accurate illegal.invalid scope
names.
@FichteFoll
Copy link
Collaborator Author

Just a heads-up that I fixed the merge conflicts.

@wbond wbond merged commit c5947de into sublimehq:master Feb 8, 2019
@FichteFoll FichteFoll deleted the javascript_json-meta-scopes branch February 9, 2019 14:43
moondi added a commit to moondi/sublime-settings that referenced this pull request May 7, 2019
@moondi
fix: adapt JSON colouring to the recent changes in the syntax defintion
2c46913 
This may require more reworking down the road (to actually follow proper semantics), but this works for now

Additional context here:
- sublimehq/Packages#862
deathaxe pushed a commit to deathaxe/sublime-packages that referenced this pull request Jun 9, 2019
@FichteFoll @wbond
[JSON] Meta scopes for mapping and sequence (sublimehq#862)
3d85caa 
* [JSON] Move value context to the top
* [JSON] Use `meta.key` and `meta.value` scopes
* [JSON] Use more general "sequence" and "mapping"
* [JSON] meta.{key,value}-> meta.mapping.{key,value}
* [JSON] Use prototype context
* [JSON] Don't stack meta.mapping and *.{key,value}
@keith-hall keith-hall mentioned this pull request Sep 3, 2019
Merged
FichteFoll added a commit to SublimeText/PackageDev that referenced this pull request Sep 20, 2019
@FichteFoll
Settings: Remove pre-3.2 JSON scope from selector
384bc66 
Removed in sublimehq/Packages#862 and eventually
released to the stable channel with the 3.2 release.
@jwortmann jwortmann mentioned this pull request Aug 24, 2020
Open
@keith-hall keith-hall mentioned this pull request Nov 26, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@FichteFoll @wbond