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

[JSON] Rewrite syntax #3097

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[JSON] Rewrite syntax #3097

wants to merge 1 commit into from

Conversation

jrappen
Copy link
Contributor

@jrappen jrappen commented Oct 21, 2021

  • Using inheritance split up JSON.sublime-syntax into:
    • JSON.sublime-syntax with scope:source.json
    • JSONC.sublime-syntax with scope:source.json.jsonc
    • JSON5.sublime-syntax with scope:source.json.json5
    • JSON_dotNET.sublime-syntax with scope:source.json.json-dotnet
  • Add many more file extensions for JSON & JSONC:
    • add doc links to extensions where applicable as a reference to be
      able to more quickly verify that they (still) use said syntax
      flavor
  • JSON:
    • Make use of newer syntax features including those only available
      in version: 2 syntaxes
    • Make use of variables (with optimizations provided by @deathaxe
      and regex patterns provided by @Thom1729)
    • Context names now more closely match the naming scheme of other
      (recently re-written) default syntaxes
    • (correctly formatted) JSON code can now be prettified or minified
      via the context menu or the command palette. JSON code can
      optionally be auto-prettified on pre save events.
    • highlight leading, trailing & multiple commas as invalid
    • only allow exactly one structure (object, array) or value
      (constant, number, string) at top level (thanks to @keith-hall)
    • links (meta.link.inet) and email addresses (meta.link.email) are
      scoped the same as in Markdown (thanks to @deathaxe)
  • JSONC:
  • JSON5:
    • explicitly pos numbers, hexadecimal ints, Infinity and NaN
    • single quoted strings
    • more escape chars for strings
    • ECMA identifierName as object keys (regexes thanks to @Thom1729)
      • scoped as plain unquoted strings (thanks to @Thom1729)
      • support string interpolation (thanks to @deathaxe)
    • line continuation in strings (with tests thanks to @keith-hall)
  • JSON.NET:
  • Objects:
    • Highlighting speed improvements for empty objects (thanks to
      @FichteFoll)
    • Make mapping.* contexts more modular
  • Arrays:
    • Highlighting speed improvements for empty arrays (thanks to
      @FichteFoll)
  • Numbers:
    • Correctly scope number signs with constant.numeric.sign instead
      of keyword.operator.arithmetic
    • Significantly improve number highlighting (thanks to @deathaxe)
  • Completions:
    • completions have been added for language constants, including kind info
      and details (with links to docs)
      • null, false, true for JSON
      • Infinity and NaN for JSON5
  • Settings:
    • a default_extension is now set for all JSON flavors
  • Symbol index:
    • with an object structure at the top-level, only top-level keys
      within now show up in the index (including tests for symbols and
      syntax)
  • Tests:
    • test files now test the base scope
    • Significantly extend tests to cover more parts of the syntaxes
    • Split original test file into logical parts
    • Add indentation tests for:
      • json, json5 & jsonc
      • mapping (objects), sequence (arrays)
    • Add symbols tests for:
      • top-level keys of object structures (thanks to deathaxe)
      • languages: json, json5 & jsonc
    • Fix tests for meta.mapping meta.mapping.*
  • Leave JSON headers in Markdown as json only, but split up fenced
    code blocks into json, json5 & jsonc to behave similarly to
    GitHub Flavored Markdown

BREAKING CHANGES:

Co-authored-by: Ashwin Shenoy Ultra-Instinct-05@users.noreply.github.com
Co-authored-by: Jack Cherng jfcherng@users.noreply.github.com
Co-authored-by: Janos Wortmann jwortmann@users.noreply.github.com
Co-authored-by: Jon Skinner jps@sublimetext.com
Co-authored-by: FichteFoll FichteFoll@users.noreply.github.com
Co-authored-by: Keith Hall keith-hall@users.noreply.github.com
Co-authored-by: Michael B. Lyons michaelblyons@users.noreply.github.com
Co-authored-by: Rafał Chłodnicki rchl@users.noreply.github.com
Co-authored-by: Thomas Smith Thom1729@users.noreply.github.com
Co-authored-by: Will Bond wbond@users.noreply.github.com
Co-authored-by: deathaxe deathaxe@users.noreply.github.com

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

⚠️ This is a first step to show my current work in progress.

There is still a lot of work to do. Please comment below with suggestions.

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

As @jskinner requested in #285, this PR applies JSONC to *.json files by default to not confuse beginners too much who do not know how to set a syntax for a view.

@jrappen jrappen marked this pull request as draft October 21, 2021 00:18
@michaelblyons
Copy link
Collaborator

What do you think about using source.json and source.json.jsonc? I worry a little that moving the primary *.json association to source.jsonc will break a bunch of people and packages' configuration scopes.

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

Sounds reasonable.

I haven't looked at other packages that either use JSON or are affected by this change, yet.

@jfcherng
Copy link
Collaborator

jfcherng commented Oct 21, 2021

I haven't looked at other packages that either use JSON or are affected by this change, yet.

Like I have one in my custom keybindings via selector context.

    // plugin: JsPrettier
    {
        "keys": ["ctrl+alt+f"],
        "command": "js_prettier",
        "context": [
            {
                "key": "selector",
                "operator": "equal",
                "operand": "text.html.basic | text.html.markdown | text.html.vue | source.js | source.json | source.css | source.scss | source.less | source.ts | source.tsx | source.yaml"
            }
        ]
    },

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

Ok, follow up question, should I change stuff like:

scope: comment.line.double-slash.jsonc

to

scope: comment.line.double-slash.json.jsonc

for JSONC.sublime-syntax?

@michaelblyons
Copy link
Collaborator

Ok, follow up question, should I change stuff like:

scope: comment.line.double-slash.jsonc

to

scope: comment.line.double-slash.json.jsonc

for JSONC.sublime-syntax?

That's disputed. One camp says "yes." One camp says "no." One camp actually says "Change the last . to a -."

In this instance, I am in the "keep it as .jsonc" camp. Closest thing in default packages is the GitHub markdown extensions.

@deathaxe
Copy link
Collaborator

Agree. The backward compatibility concerns are mainly tarteting the syntax's main scope here.

If a derived/extending syntax adds more patterns, it's ok to only add it's trailing part of a main scope.

So I am also in the "keep it as .jsonc" camp.

@deathaxe
Copy link
Collaborator

JSON with Comments (jsonc) is just an extension of JSON. I don't therefore see any value in adding a JSON (Basic) which JSON is derived from by just adding some file extensions.

JSON/JSONC.sublime-syntax Outdated Show resolved Hide resolved
@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

JSON with Comments (jsonc) is just an extension of JSON. I don't therefore see any value in adding a JSON (Basic) which JSON is derived from by just adding some file extensions.

I'm still undecided if I should add more flavors in this step ... or do that later. This was just some preparation for (possibly) later.

@deathaxe
Copy link
Collaborator

I'm still undecided if I should add more flavors in this step ... or do that later. This was just some preparation for (possibly) later.

Haven't investigated any other flavours nor being aware of them, but does that change anything?

The default JSON.sublime-syntax could still be the base for everything with its source.json main scope, no? It shouldn't even hurt to add the .basic and keep it as is.

So we'd end up in:

file scope
JSON.sublime-syntax source.json.basic
JSONC.sublime-syntax source.json.jsonc
JSON (<flavor>).sublime-syntax source.json.<flavor>

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

Probably not. Unless I have to extract bits from JSON (Basic) to JSON. It's easier to merge than separate them later.

@michaelblyons
Copy link
Collaborator

It shouldn't even hurt to add the .basic and keep it as is.

What happens when another syntax - include: scope:source.json if you change the root scope to source.json.basic? Will something be included? If so, what?

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

If so, what?

In that case I'd assume nothing.

@deathaxe
Copy link
Collaborator

You are right @michaelblyons. Nothing is included then. So it's not an option.

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

Could use this instead, though:

- include: scope:Packages/JSON/JSON.sublime-syntax

unless I'm mistaken. I have a vague memory seeing this somewhere.

@michaelblyons
Copy link
Collaborator

Could use this instead, though:

- include: scope:Packages/JSON/JSON.sublime-syntax

unless I'm mistaken. I have a vague memory seeing this somewhere.

I know you can do this within a package. I believe Git Formats does so. You may be able to do it between packages (e.g. Markdown front matter).

But switching away from source.json would still break other people's packages all over the internet.

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

Sometimes, breaking stuff is the way forward. No need for that here, though.

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

On the other hand, the package you linked from across the internet I will definitely break, cause he'd have to switch to scope:source.json.jsonc#prototype as JSON won't have that (i.e. comments) anymore.

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

Does anybody know of any JSON flavors that do accept decimal floats with leading or trailing periods in the base?

In basic JSON they apparently are illegal (and weren't scoped as such, yet).

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

⚠️ I have addressed and resolved all comments above, except for the one about meta_include_prototype.


Decimal floats are now forced to a more explicit format.

@jrappen
Copy link
Contributor Author

jrappen commented Oct 21, 2021

Thoughts on embed: scope:source.json in:

  • LaTeX
  • Markdown
  • Perl
  • PHP Source

and push: scope:source.json in:

  • PHP Source

anyone?

@jrappen
Copy link
Contributor Author

jrappen commented Nov 6, 2021

I personally believe that if someone chooses JSON as syntax, it should be highlighted as such. The current situation does not allow this using a default installation of Sublime Text. This was the starting point for me as well as FichteFoll in the issue linked in the opening post.

Deviating from the current situation and introducing other flavors of JSON will cause breaking changes. Either one way or another.


We can either (1) keep JSONC be a JSON syntax and introduce a JSON Basic (to make inheritance work) and JSON Strict (as a true JSON syntax). Any monkey patching around wrongly labeled syntaxes we avoid now in core and for any third parties will simply be defered to the future where third parties will have to address JSON Strict instead of JSON. Of course they will figure it out, but maybe not right away. Maybe this is less overall cost of time. Maybe not. I see this as offloading past problems to others in the future. And continuing to do so, as this has been an issue for a while and got us here in the first place. We can agree to disagree on this, I'm just stating my point.

Or we can (2) cause a bit of a stir now but be at peace for a long while afterwards. Hence why we can test this in the dev release channel for a while, before this hits stable. I'd assume package authors of frequently used packages are on the dev channel anyway.

We can also wait with this, until there is a longer cycle, if now is not a good time.


Not sure why you need to stress that the status bar label for the JSONC syntax is so ugly currently. I already agreed to change it to just JSONC. You can auto hide it while typing if desired. No big deal.


The JSON syntax exists for files with patterns listed in file_extensions, but you knew that. Why are you asking? If we remove the JSON syntax, what is the remaining point of this PR? Just adding JSON5?


If the bright red colors are such a bother, we have a few options. We can (1) either keep it and you add an override to Mariana to dim your reds, or (2) SublimeHQ will do it for you (which will never happen as we both know) or I (3) remove all illegal highlighting from JSON and add a linter that runs on the json module which ships with the Python included with Sublime and we have underlines or outlines - or whatever choice we agree upon - to highlight invalid regions (in this case). Or (4) I just remove all illegal highlighting, but then there really is no big difference between the highlighting of JSON vs JSONC.

Color schmes should follow syntaxes and (personally) I don't mind the red highlight of (temporarily or not but nonetheless) illegal code.

Because we are discussing:

  • trailing periods within floats
  • and trailing commas within structures (objects and arrays)

the user experience is flashing if we assume the user types fast. For you the flashing is more important (and disturbing) than is the highlighting of illegal code for me. We can again agree to disagree here, perfectly fine.

The problem for me as an author of the syntax file is though, that I cannot differentiate between "user typing" and "user done typing". Unless there is a way of "defering the highlight until after typing".

My suggestion is having a separate JSON syntax and writing it as it should be, not as the current user with old habits is used to. We can again agree to disagree.

If most users agree with youk then we should agree to close this PR as it will not benefit you or them. Hence my suggestion.


Which is fine with me, I can use this locally. I don't see this as having wasted my time. I have personal use for this.

@keith-hall
Copy link
Collaborator

My 2 cents on this one issue about trailing commas being highlighted as invalid (while typing/not typing - we have no distinction as has been said already):

They are illegal in "strict JSON", and should be highlighted as such. Yes, it is possible that it could be annoying for users while typing. But:

  • the benefits outweigh the drawbacks - it will likely save lots of trouble with people new to JSON who don't realize that trailing commas are illegal. "ST allows it for it's settings, but it's not working in my [language X] application. hmm... ah, I see that the syntax is different - JSONC vs JSON. Probably that's got something to do with it..."
  • if we scope multiple consecutive commas distinctly from one trailing comma, color schemes/users can choose the behavior they want for trailing commas. ST isn't a "one size fits all" out of the box - it's designed to be customized.
  • users can use snippets etc. to minimize the "flashes of red" by avoiding having trailing commas while typing at all
  • how many people actually write JSON (and I mean "JSON (strict)" here I guess ;)) by hand these days?

@jrappen
Copy link
Contributor Author

jrappen commented Nov 6, 2021

I would agree with you, Janos, if this was a constant flicker like if you have two surfaces in the same plane in 3D and your maschine is constantly trying to render both. This is a single flash though of one single character which goes away after typing one (within the float) or two (within an array or object) additional characters.

For this reason I find this acceptable.

@jrappen
Copy link
Contributor Author

jrappen commented Nov 17, 2021

Hello guys, I didn't forget this. Simply didn't have the time to take care of this during the last couple days. I'll update this towards the weekend, as time permits.

@jrappen
Copy link
Contributor Author

jrappen commented Dec 2, 2021

⚠️ Status update


Some TODOs remaining before I'll open this for review:

  • JSON object: leading separators
  • JSON object: trailing commas
  • JSON numbers: decimal floats with leading and trailing periods
  • JSON5: valid whitespace

@keith-hall
Copy link
Collaborator

I was going to ask whether you plan to work on scoping for dates, but then realized that what I've seen in the wild is not standard in any official JSON variant: https://davidsekar.com/javascript/converting-json-date-string-date-to-date-object

@michaelblyons
Copy link
Collaborator

I was going to ask whether you plan to work on scoping for dates, but then realized that what I've seen in the wild is not standard in any official JSON variant: https://davidsekar.com/javascript/converting-json-date-string-date-to-date-object

Haha. The old Microsoft JSON date format, leading to one of my Stack Overflow bookmarks.

@jrappen
Copy link
Contributor Author

jrappen commented Dec 4, 2021

I was going to ask whether you plan to work on scoping for dates ...

I could still add them, though. I believe it adds value?

@jrappen
Copy link
Contributor Author

jrappen commented Dec 9, 2021

@keith-hall Could you paste me a few links with references for your last request? I'd appreciate it.

I won't be adding your request to the JSON flavors I currently have (JSON, JSONC, JSON5), but rather add another flavor. I believe it's less confusing to add another flavor than to add this kind of stuff to the ones we already have and where it's not expected.

Are we talking about this?

@keith-hall
Copy link
Collaborator

@deathaxe
Copy link
Collaborator

The current implementation causes trouble with partial json in Markdown fenced code blocks.

grafik

Hence I am not convinced of adding too much of such over eager illegal syntax highlighting stuff.

@jrappen
Copy link
Contributor Author

jrappen commented Dec 12, 2021

causes trouble with partial json in Markdown fenced code blocks

I've had this on my checklist for a while.

I initially thought it might make sense to have a (JSON) "snippet" syntax, since JSON is a comparatively simple syntax.

@deathaxe
Copy link
Collaborator

JSON snippet syntax? There are already enought dialects. Adding more doesn't make sense. I'd rather relax existing JSON to not complain about incomplete globals.

@jrappen
Copy link
Contributor Author

jrappen commented Apr 5, 2022

Should I meta-scope links and email addresses in value strings as in the Markdown re-write?

@jrappen jrappen force-pushed the fix-json branch 3 times, most recently from 132755a to 83806c3 Compare May 31, 2022 23:53
- Using inheritance split up `JSON.sublime-syntax` into:
    - `JSON.sublime-syntax` with `scope:source.json`
    - `JSONC.sublime-syntax` with `scope:source.json.jsonc`
    - `JSON5.sublime-syntax` with `scope:source.json.json5`
    - `JSON_dotNET.sublime-syntax` with `scope:source.json.json-dotnet`
- Add many more file extensions for `JSON` & `JSONC`:
    - add doc links to extensions where applicable as a reference to be
      able to more quickly verify that they (still) use said syntax
      flavor
- JSON:
    - Make use of newer syntax features including those only available
      in `version: 2` syntaxes
    - Make use of `variables` (with optimizations provided by @deathaxe
      and regex patterns provided by @Thom1729)
    - Context names now more closely match the naming scheme of other
      (recently re-written) default syntaxes
    - (correctly formatted) JSON code can now be prettified or minified
      via the context menu or the command palette. JSON code can
      optionally be auto-prettified on pre save events.
    - highlight leading, trailing & multiple commas as invalid
    - only allow exactly one structure (object, array) or value
      (constant, number, string) at top level (thanks to @keith-hall)
    - links (`meta.link.inet`) and email addresses (`meta.link.email`) are
      scoped the same as in Markdown (thanks to @deathaxe)
- JSONC:
    - highlight some files by default as `JSONC` (as decided by
      @jskinner in sublimehq#285)
    - highlight leading & multiple commas as invalid, trailing as valid
    - scope empty block comments as such
    - support syntax based folding of ST4131+,
      compare sublimehq#3291
- JSON5:
    - explicitly pos numbers, hexadecimal ints, Infinity and NaN
    - single quoted strings
    - more escape chars for strings
    - ECMA identifierName as object keys (regexes thanks to @Thom1729)
        - scoped as plain unquoted strings (thanks to @Thom1729)
        - support string interpolation (thanks to @deathaxe)
    - line continuation in strings (with tests thanks to @keith-hall)
- JSON.NET:
    - support requested by @keith-hall,
      built with feedback from @michaelblyons
- Objects:
    - Highlighting speed improvements for empty objects (thanks to
      @FichteFoll)
    - Make `mapping.*` contexts more modular
- Arrays:
    - Highlighting speed improvements for empty arrays (thanks to
      @FichteFoll)
- Numbers:
    - Correctly scope number signs with `constant.numeric.sign` instead
      of `keyword.operator.arithmetic`
    - Significantly improve number highlighting (thanks to @deathaxe)
- Completions:
    - completions have been added for language constants, including kind info
      and details (with links to docs)
        - `null`, `false`, `true` for JSON
        - `Infinity` and `NaN` for JSON5
- Settings:
    - a `default_extension` is now set for all JSON flavors
- Symbol index:
    - with an object structure at the top-level, only top-level keys
      within now show up in the index (including tests for symbols and
      syntax)
- Tests:
    - test files now test the base scope
    - Significantly extend tests to cover more parts of the syntaxes
    - Split original test file into logical parts
    - Add indentation tests for:
        - `json`, `json5` & `jsonc`
        - `mapping` (objects), `sequence` (arrays)
    - Add symbols tests for:
        - top-level keys of object structures (thanks to deathaxe)
        - languages: `json`, `json5` & `jsonc`
    - Fix tests for `meta.mapping meta.mapping.*`
- Leave `JSON` headers in `Markdown` as `json` only, but split up fenced
  code blocks into `json`, `json5` & `jsonc` to behave similarly to
  `GitHub Flavored Markdown`

BREAKING CHANGES:

- JSON does not have values that can be set via an inline calculation
  with the help of operators, but only simple number values. Scopes for
  number signs have changed from being `keyword.operator.arithmetic` to
  `constant.numeric.sign`. Color scheme authors should add this, should
  it be missing.
- The `JSON.sublime-syntax` now marks comments as `invalid`, third party
  plugin authors should instead target `JSONC.sublime-syntax` to keep
  the user experience as-is.
- Indexed symbols (i.e. top-level keys in JSON object structures) are
  scoped as `source.json meta.mapping.key - (meta.mapping.value meta.mapping.key | meta.sequence.list meta.mapping.key)`.
  Color scheme authors should add special highlighting to differentiate
  them from other keys.

- fix sublimehq#285
- address sublimehq#421 (thanks to @FichteFoll)
- address sublimehq#481 to remove incompatible regex patterns according
  to @wbond
- address sublimehq#757 to fix line comments for `JSONC` (thanks to
  @keith-hall)
- address sublimehq#2430 using sort-order (as requested by @deathaxe)
- address sublimehq#2711 with regards to `constant.language.null`
  vs. `constant.language.empty` (thanks to @FichteFoll)
- address sublimehq#2852 to fix scopes of curly braces & square
  brackets in `JSON` (thanks to @Thom1729)
- address sublimehq#3228 to fix `punctuation.separator` scopes,
  compare sublimehq#3270
- address sublimehq/sublime_text#3154 and add symbol tests

Co-authored-by: Ashwin Shenoy <Ultra-Instinct-05@users.noreply.github.com>
Co-authored-by: Jack Cherng <jfcherng@users.noreply.github.com>
Co-authored-by: Janos Wortmann <jwortmann@users.noreply.github.com>
Co-authored-by: Jon Skinner <jps@sublimetext.com>
Co-authored-by: FichteFoll <FichteFoll@users.noreply.github.com>
Co-authored-by: Keith Hall <keith-hall@users.noreply.github.com>
Co-authored-by: Michael B. Lyons <michaelblyons@users.noreply.github.com>
Co-authored-by: Rafał Chłodnicki <rchl@users.noreply.github.com>
Co-authored-by: Thomas Smith <Thom1729@users.noreply.github.com>
Co-authored-by: Will Bond <wbond@users.noreply.github.com>
Co-authored-by: deathaxe <deathaxe@users.noreply.github.com>
@michaelblyons
Copy link
Collaborator

Just so you know, the @ mention in the commit description means we're getting an email for every force push.

@jrappen
Copy link
Contributor Author

jrappen commented Jun 1, 2022

Sure, but if I use bold text, it wont be linked up correctly after merge.

@UltraInstinct05
Copy link
Contributor

I am not really sure if it's good idea to ship minification/prettification plugins for JSON with the Default package. Would be a really nice excuse for people to complain about why other syntax packages don't ship with such. IMHO, best suited for a 3rd party package. Maybe LSP-json even has such capabilities, but haven't checked.

@TerminalFi
Copy link
Contributor

Not a fan of built in minify and prettify.It is adding another entry point for UI sluggishness. I imagine this being shipped and then issues being filed for Sublime being slow when prettifying or minifying json.

This is better suited for a plug-in.

@jrappen
Copy link
Contributor Author

jrappen commented Feb 27, 2023

I'll update the PR when I have some more time later this week ... or so.

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.

Separate JSON and "JSON with comments" syntaxes?
9 participants