After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 660194 - [New lang] JavaScript Object Notation
[New lang] JavaScript Object Notation
Status: RESOLVED FIXED
Product: gtksourceview
Classification: Platform
Component: Syntax files
git master
Other All
: Normal enhancement
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on:
Blocks:
 
 
Reported: 2011-09-26 23:29 UTC by Wa
Modified: 2011-10-09 11:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
new lang: JSON (4.82 KB, patch)
2011-09-26 23:29 UTC, Wa
needs-work Details | Review
new lang: JSON (fixed) (6.38 KB, patch)
2011-10-09 09:17 UTC, Wa
none Details | Review

Description Wa 2011-09-26 23:29:05 UTC
Created attachment 197535 [details] [review]
new lang: JSON

Best implementation I can muster for JSON. There are a couple notes in the file regarding future possibilities should gtksourceview be updated to allow such things.

This is very strict JSON. There are no comments allowed etc.

Where it can, this is built to reference javascript.lang's styles for what I'd consider proper cascading for themes.
Comment 1 Paolo Borelli 2011-09-27 07:15:42 UTC
Review of attachment 197535 [details] [review]:

Hi, thanks for the lang file (and also for the other ones you attached), I'll start from json since I am more familiar with it.

Some general comments that apply to all lang files:
 - please use two-spaces indetation insteaad of tabs
 - include the mimetype metadata only if the type is included in freedesktop's shared-mime-info database, otherwise should be omitted
 - the file should be added to po/POTFILES.in for translation
 - if possible add small snippet of the language to test/testfiles.sh

With regard to json specifically, I would expect it to be composed exclusively by including specific contexts from javascript.lang, so for instance you would have <include><context ref="js:boolean"/></include> instead of redefining the boolean context. If you want to reuse a js context but allow to override the style, you can use <context ref="js:boolean" style-ref="boolan">.

The above would also mean changing/improving javascript.lang by adding there the contexts that are missing (if they make sense for javascript).

You would also need to remove the *.json glob from javascript.lang
Comment 2 Wa 2011-09-27 20:24:41 UTC
Ok I will begrudgingly change them to spaces~ :p

And right! I had included those but I was experimenting with once-only to only allow one value per entry (to no avail) and forgot to change it back.

Also, strings and escapes are more limited than official JavaScript so they'll stay in JSON. As for objects and arrays, well, those delimiters have other uses in JS too, it'd be incredibly more complex.

Didn't even see that glob in javascript.lang :]

Patch later today I'm sure.
Comment 3 Wa 2011-10-08 04:05:20 UTC
Sorry about the wait, my computer died and I've been setting up a new one.

Anyway, could you tell me about the test file thing? Should I include highlighted errors into the test? Is this automated so that I should be watching out for something?
Comment 4 Wa 2011-10-09 09:17:01 UTC
Created attachment 198646 [details] [review]
new lang: JSON (fixed)

Alright this should cover it. Changed it to be of type Others (it's not really Markup after all..). Removed *.json glob from javascript.lang, and added *.node as that's used by Node.JS for JavaScript files.
Comment 5 Paolo Borelli 2011-10-09 09:45:06 UTC
Review of attachment 198646 [details] [review]:

Looks great to me now. You are actually missing the changes to Makefile.am to make sure the file is installed, but I'll amend the patch, test it and commit
Comment 6 Paolo Borelli 2011-10-09 11:45:56 UTC
I fixed up a few things and committed. Thanks!