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

[JavaScript] Move JSON out of the JavaScript package. #1771

Closed
Thom1729 opened this issue Nov 26, 2018 · 5 comments
Closed

[JavaScript] Move JSON out of the JavaScript package. #1771

Thom1729 opened this issue Nov 26, 2018 · 5 comments

Comments

@Thom1729
Copy link
Collaborator

The JSON syntax definition, tmPreferences, and tests are inside the JavaScript directory. This means that a user can't disable the JavaScript package (e.g. when using a third-party package) without also disabling the core JSON syntax (and vice versa).

JSON had its roots in JavaScript, but it is nowadays used for all sorts of things. I think that it would make sense to promote JSON to its own default package. There is no module named "JSON" in the Package Control channel, so there should be no serious risk of conflict.

Some third-party code could be hardcoding the location of JSON.sublime-syntax. If this is a concern, we could leave a stub at that location:

%YAML 1.2
---
name: JSON (Compatibility redirect)
scope: ''
hidden: true
contexts:
  main:
    - match: ''
      embed: 'Packages/JSON/JSON.sublime-syntax'
      escape: \B\b # Never
@FichteFoll
Copy link
Collaborator

Also worth thinking about #285 when this is considered.

@deathaxe
Copy link
Collaborator

All included sublime-syntax files I've seen so far have been used without path. Something like - include: "blabla.sublime-syntax". The only situations I saw paths were in syntax test files.

In case of a stub file, I think include would just be enough?

%YAML 1.2
---
name: JSON (Compatibility redirect)
scope: ''
hidden: true
contexts:
  main:
    - include: 'Packages/JSON/JSON.sublime-syntax'

@FichteFoll
Copy link
Collaborator

FichteFoll commented Nov 30, 2018 •
edited
Loading

It would still need a base scope, but other than that I suppose it should work. Since "JSON" comes after "JavaScript" the new one would take priority.

deathaxe pushed a commit to deathaxe/sublime-packages that referenced this issue Dec 8, 2018
[JavaScript] Move JSON out of the JavaScript package
593dbbd 
As suggested in issue sublimehq#1771 this commit moves the JSON syntax into a
dedicated package in order to be able to disable either JavaScript or
JSON independently.

This commit does not yet create the suggested compatibility
redirection within the JavaScript package.

The decision about it is left to the core devs right now

1. as it would cause a duplicated syntax file
2. a quick search within packagecontrol.io did not reveal any packages
   depending on the builtin JavaScript/JSON.sublime-syntax
3. the location of the sublime-syntax file should not be too important
   if it is imported via `scope:source.json` or its name
   `JSON.sublime-syntax` only. Conflicts would be caused only if it was
   imported via `Packages/JavaScript/JSON.sublime-syntax` which is easy
   to fix.
@deathaxe deathaxe mentioned this issue Dec 8, 2018
Merged
deathaxe pushed a commit to deathaxe/sublime-packages that referenced this issue Feb 8, 2019
[JavaScript] Move JSON out of the JavaScript package
d7bf5d6 
As suggested in issue sublimehq#1771 this commit moves the JSON syntax into a
dedicated package in order to be able to disable either JavaScript or
JSON independently.

This commit does not yet create the suggested compatibility
redirection within the JavaScript package.

The decision about it is left to the core devs right now

1. as it would cause a duplicated syntax file
2. a quick search within packagecontrol.io did not reveal any packages
   depending on the builtin JavaScript/JSON.sublime-syntax
3. the location of the sublime-syntax file should not be too important
   if it is imported via `scope:source.json` or its name
   `JSON.sublime-syntax` only. Conflicts would be caused only if it was
   imported via `Packages/JavaScript/JSON.sublime-syntax` which is easy
   to fix.
wbond pushed a commit that referenced this issue Nov 15, 2019
@deathaxe @wbond
[JavaScript] Move JSON out of the JavaScript package (#1805)
5ce2296 
Note from wbond: initial work on splitting JSON happened in 9c44e60, this ties up some loose ends.

As suggested in issue #1771 this commit moves the JSON syntax into a
dedicated package in order to be able to disable either JavaScript or
JSON independently.

This commit does not yet create the suggested compatibility
redirection within the JavaScript package.

The decision about it is left to the core devs right now

1. as it would cause a duplicated syntax file
2. a quick search within packagecontrol.io did not reveal any packages
   depending on the builtin JavaScript/JSON.sublime-syntax
3. the location of the sublime-syntax file should not be too important
   if it is imported via `scope:source.json` or its name
   `JSON.sublime-syntax` only. Conflicts would be caused only if it was
   imported via `Packages/JavaScript/JSON.sublime-syntax` which is easy
   to fix.
@FichteFoll
Copy link
Collaborator

Can be closed since #1805 has been merged.

deathaxe added a commit to deathaxe/sublime-packages that referenced this issue Dec 18, 2020
@deathaxe
Close issues
1665b2c 
Closes sublimehq#1555 (fixed)
Closes sublimehq#1671 (fixed by core)
Closes sublimehq#1696 (fixed)
Closes sublimehq#1699 (fixed)
Closes sublimehq#1771 (fixed)
Closes sublimehq#2034 (fixed)
Closes sublimehq#2329 (fixed)
Closes sublimehq#2550 (invalid)
@deathaxe deathaxe mentioned this issue Dec 18, 2020
Closed
@wbond
Copy link
Member

wbond commented Jan 14, 2021

This has been completed

@wbond wbond closed this as completed Jan 14, 2021
mitranim pushed a commit to mitranim/Packages that referenced this issue Mar 25, 2022
@deathaxe @mitranim
[JavaScript] Move JSON out of the JavaScript package (sublimehq#1805)
966ade2 
Note from wbond: initial work on splitting JSON happened in 9c44e60, this ties up some loose ends.

As suggested in issue sublimehq#1771 this commit moves the JSON syntax into a
dedicated package in order to be able to disable either JavaScript or
JSON independently.

This commit does not yet create the suggested compatibility
redirection within the JavaScript package.

The decision about it is left to the core devs right now

1. as it would cause a duplicated syntax file
2. a quick search within packagecontrol.io did not reveal any packages
   depending on the builtin JavaScript/JSON.sublime-syntax
3. the location of the sublime-syntax file should not be too important
   if it is imported via `scope:source.json` or its name
   `JSON.sublime-syntax` only. Conflicts would be caused only if it was
   imported via `Packages/JavaScript/JSON.sublime-syntax` which is easy
   to fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Close issues deathaxe/sublime-packages
4 participants
@wbond @FichteFoll @deathaxe @Thom1729