JSON Language

[CupOfTea696]

Added JSON support to prism.

[CupOfTea696]

Alternatively, 'variable' could be used instead of 'property', which would look nicer in my opinion but is semantically incorrect.

[LeaVerou]

See my comments on #369, they remain valid.
Btw you can update pull requests, you don’t need to close them and send a new one. :)

[CupOfTea696]

Right, sorry I didn't know you could do that.
First of, the reason I added JSON is because the highlighting didn't work properly when just using javascript, I got a TypeError on var match = pattern.exec(str);
The reason I didn't extend javascript is because javascript has a lot more things in it than JSON has, so they would have to be removed from the definition and I wasn't sure that was possible. Regardless, it doesn't seem to make sense to me if you extend something and than scratch at least 50% of the parent.
Secondly, I hadn't realised that single quotes strings weren't allowed in JSON, so I do need yo update that.
Lastly, the reason why I didn't just use the i flag for the booleans and null is because things like faLSE aren't semantically correct, although it probably would make sense to highlight them anyway.

Also I'm still debating on wether I should use 'property' or 'variable' for the properties. It might be a good idea to just use property and then add some css for .language-json .token.property

[apfelbox]

Lastly, the reason why I didn't just use the i flag for the booleans and null is because things like faLSE aren't semantically correct, although it probably would make sense to highlight them anyway.

Neither is NULL. So you should either include all cases or just the one in the spec (null).

Also I'm still debating on wether I should use 'property' or 'variable' for the properties. It might be a good idea to just use property and then add some css for .language-json .token.property

Or define it as alias (@LeaVerou did we agree on using alias for things like this)?

For the implementation: I would probably go with extending javascript, too. At least for not having to duplicate a lot of code.
Prism is no lint tool - so accepting invalid JSON is perfectly fine here.

If there are issues with using javascript with JSON data: all right - let's fix it. 😃

[uranusjr]

There is actually one extreme edge case, as described in this post, in which the JavaScript rules wouldn’t highlight JSON properly, while this implementation does. This can be fixed by extending JavaScript instead, obviously, but that seems a bit backwards IMO to include so much (C-like and JavaScript) for so few syntax features.

[CupOfTea696]

If you do want me to move the JSON to a javascript extension, you'd have to tell me how to remove the things it inherits that it doesn't need. But I completely agree with @uranusjr on this one. I just checked and JSON would only inherit 2 definitions (number and boolean), which really doesn't seem to make it worthwhile.

[CupOfTea696]

By the way, is there a way I can safely rename my branch, because I get a build warning email with every commit because the branch is still named gh-pages.

[LeaVerou]

By the way, is there a way I can safely rename my branch, because I get a build warning email with every commit because the branch is still named gh-pages.

That’s a known issue that will eventually be solved once we separate master and gh-pages… That would also satisfy the bower crowd. The problem is that currently, I'm not sure how to make such a split in a DRY manner.

[LeaVerou]

If you do want me to move the JSON to a javascript extension, you'd have to tell me how to remove the things it inherits that it doesn't need.

delete Prism.languages.foo;

I just checked and JSON would only inherit 2 definitions (number and boolean), which really doesn't seem to make it worthwhile.

You’re right, it’s not worth it just for two tokens, one of which is rather simple. @apfelbox what do you think?

Also, IMO we should merge JSON and JSONP, either by having both on the same file, or in the same language definition too (any opinions?)

[CupOfTea696]

Same file would make sense, but I would recommend keeping the distinction between the two. Awaiting your decision before making additional changes.

[apfelbox]

I would argue that loading too much doesn't harm a lot. The complete javascript markup is < 1kB, compile time in the browser is practically non-existant.

The only difference between JS and JSON seems to be (according to the link @uranusjr posted) native support for UTF-8 in all strings - which we could easily fix by just allowing them in JS too (again: Prism is a forgiving highlighter, no merciless linter).

But:
As my main issue (code duplication) doesn't stand (as @CupOfTea696 noted) I would vote for just releasing it as a standalone JSON + JSONP language. 👍

[apfelbox]

(as a single language definition json with alias for jsonp)

[LeaVerou]

Prism is a forgiving highlighter, no merciless linter

Excellent point. This should be in all bold on the contributor readme I think :p

I think the main issue between sharing strings in these two defs is that JSON doesn't allow single quoted strings (which also makes its regex easier, as no backreference is needed)

[CupOfTea696]

Prism is a forgiving highlighter, no merciless linter

Since we are being so forgiving I really can't deny that putting the jsonp in the json definition and just making it an alias is a bad idea :)

[apfelbox]

Prism.languages.jsonp = Prism.languages.json;

[CupOfTea696]

oh wow im stoopid :p

[AViscatanius]

Rename a branch? I hope it's easy

[LeaVerou]

What happened here? Why did this stall?

[chris-martin]

JSON is the only thing I'm using Prism for, so I'm not sure whether this is a bug in this plugin or in the library, but FWIW: This doesn't work well with strings that contain numbers.

[Golmote]

@chris-martin: This component has not made it into Prism yet. I think you'd better use the JS component to highlight your JSON, for now, as it seems to work properly:

[LeaVerou]

@CupOfTea696 Reversing the order of the number and string tokens shall fix this. Please refer to the JS language definition for the optimal order.

[CupOfTea696]

That should've fixed it.

[chris-martin]

I'm confused about how this can make a difference. Aren't JS objects unordered?

[Golmote]

In theory they are. In practice all engines follow the order in which the properties were defined. And Prism relies on this behaviour.

[uranusjr]

(See also discussion in #441)

[markhalliwell]

This works rather well.

The only thing that I see missing is that if you use the show-language plugin, it only capitalizes the first letter. This will need to be added to the Languages object in https://github.com/PrismJS/prism/blob/gh-pages/plugins/show-language/prism-show-language.js#L8

Here is a temporary work around:

  Prism.hooks.add('before-highlight', function(env) {
    var pre = env.element.parentNode;
    if (!pre || !/pre/i.test(pre.nodeName)) return;
    if (/^json/i.test(env.language)) {
      pre.setAttribute('data-language', 'JSON');
    }
  });

[nauzilus]

This is done automatically via the gulp build task: https://github.com/PrismJS/prism/blob/gh-pages/gulpfile.js#L52-L89

[darobin]

Is there any hope of this landing soonish? Highlighting with JS isn't very helpful as pretty much everything ends up being pretty much the same colour :) Also, I've been thinking of making it possible to style keys starting with @ differently so it works for JSON-LD too, if that's acceptable.

[Golmote]

@darobin I'll try to make a proper review of this PR by the end of the week.

[adalinesimonian]

Hate to be that guy (I understand this is an OSS project), but any updates on a code review? This PR has been around for over a year now.

[drodil]

Anything new to this?

[LeaVerou]

JSON doesn't have functions…

[LeaVerou]

Sorry this has taken so long. I just took a look and added a few comments.

[CupOfTea696]

@LeaVerou no worries! Applied the necessary changes.

[LeaVerou]

Merged, thanks!
@darobin I think a change to accommodate JSON-LD would be awesome!! Please submit a PR for it, I would love this.

[drodil]

Hi, good work! Is this going to be released anytime soon?

[LeaVerou]

@drodil Any merged PR is immediately available to the live site.

Btw, @CupOfTea696, I noticed you made this enabled by default. While I did merge it to include it in Prism, I do not agree with it being enabled by default. Could you or someone else please send another PR to remove option: default?

[zeitgeist87]

Could you or someone else please send another PR to remove option: default?

I've removed the default option.

[LeaVerou]

Thank you Andrea!!

[CupOfTea696]

@LeaVerou That's probably because I copied it over from javascript. Sorry about that. Probably wasn't sure on what it did and just left it that way :p