Skip to content

Loading…

Added support for JavaScript-style comments in JSON files and improved t... #804

Closed
wants to merge 1 commit into from

5 participants

@jlecomte
Yahoo Inc. member

JSON does not allow for comments... Maintaining our configuration files, which are starting to get gigantic, is therefore quickly becoming a nightmare! This PR allows for comments in JSON files in Mojito by stripping those files from JavaScript-style comments before passing them to JSON.parse. Also, this PR improves a bit the situation with JSLint, although I noticed JSLint does not like the intentional fallthrough in the switch statement... I like it, personally. We can revisit that later if it turns out to be a problem...

This is a new PR following PR #796 which was made from the develop-perf branch which went away... @lzhan confirmed in a private email that this PR was indeed needed because it seems that files with a .json extension are not parsed by the YAML parser (which may not even support JSON)

@drewfish
Yahoo Inc. member

We do intend to support comments in config files, though after 0.5.0GA.

The latest thinking is to support YAML, which is a superset of JSON, and which supports comments. (So, any file that is valid JSON is also valid YAML.) One good resource for testing what YAML supports is the online parser. (Comments start with the # character BTW.)

@jlecomte
Yahoo Inc. member

Drew, this would be great for us. What is the ETA to have Mojito fully support YAML?

@rwaldura

We're really close, but postponed it till after GA purposefully.

@rwaldura

0.5.1, seeing how close we are.
Sprint after 0.5 GA release.

@jlecomte
Yahoo Inc. member

OK, if this feature request is being scheduled for 0.5.1, you may close this PR.

@drewfish
Yahoo Inc. member

Cool. We'll probably leave this PR open for a while, until we've done planning for 0.5.1, to make sure it doesn't get lost.

@redonkulus

Not to hijack this PR but have you guys had a chance to look at JSON5? Is a superset of JSON which supports comments, multi line, trailing commas and other things. Obviously performance and other backwards compatibility features would need to be evaluated. However, it keeps the JSON format with the added commenting feature this PR introduces.

https://github.com/aseemk/json5

@jlecomte
Yahoo Inc. member

@redonkulus yes, we have. Please see PR #796 for some additional background about this thread.

@drewfish drewfish referenced this pull request
Closed

re-enable yaml support #907

@caridy caridy added a commit to caridy/mojito that referenced this pull request
@caridy caridy re-enable yaml support and accomodating multi-part ycb for applicatio…
…n.json as well. this fixes issue #804
537daf5
@caridy caridy referenced this pull request
Merged

re-enable yaml support #916

@caridy caridy closed this in c278416
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 28, 2012
  1. @jlecomte
This page is out of date. Refresh to see the latest.
Showing with 82 additions and 14 deletions.
  1. +82 −14 lib/app/addons/rs/config.js
View
96 lib/app/addons/rs/config.js
@@ -4,7 +4,7 @@
* See the accompanying LICENSE file for terms.
*/
-/*jslint anon:true, sloppy:true, nomen:true, stupid:true*/
+/*jslint anon:true, sloppy:true, nomen:true, stupid:true, plusplus:true*/
/*global YUI*/
@@ -20,7 +20,7 @@
* @class RSAddonConfig
* @extension ResourceStore.server
*/
-YUI.add('addon-rs-config', function(Y, NAME) {
+YUI.add('addon-rs-config', function (Y, NAME) {
var libfs = require('fs'),
libpath = require('path'),
@@ -28,9 +28,77 @@ YUI.add('addon-rs-config', function(Y, NAME) {
libycb = require('ycb'),
libyaml = require('js-yaml');
+
+ /**
+ * Strip JavaScript-style comments in a JSON string.
+ * @method stripCommentsInJSON
+ * @param {String}
+ * @return {String}
+ * @private
+ */
+ function stripCommentsInJSON(s) {
+ var ret = '',
+ i = 0,
+ l = s.length,
+ n,
+ j,
+ c;
+
+ while (i < l) {
+ c = s[i];
+
+ switch (c) {
+
+ // String literal...
+ case '"':
+ j = s.indexOf(c, i + 1);
+ if (j < 0) {
+ j = l;
+ }
+ ret += s.substring(i, j + 1);
+ i = j + 1;
+ break;
+
+ case '/':
+ if (i < l - 1) {
+ if (s[i + 1] === '/') {
+ // Single line comment...
+ i = s.indexOf('\n', i + 2);
+ if (i < 0) {
+ // We've reached the end.
+ i = l;
+ }
+ break;
+ } else if (s[i + 1] === '*') {
+ // Multi-line comment...
+ i = s.indexOf('*/', i + 2);
+ if (i < 0) {
+ // We've reached the end.
+ i = l;
+ } else {
+ // Skip the trailing "*/"
+ i += 2;
+ }
+ break;
+ }
+ }
+
+ // INTENTIONAL FALLTHROUGH...
+
+ default:
+ ret += s[i++];
+ break;
+ }
+ }
+
+ return ret;
+ }
+
+
function RSAddonConfig() {
RSAddonConfig.superclass.constructor.apply(this, arguments);
}
+
RSAddonConfig.NS = 'config';
Y.extend(RSAddonConfig, Y.Plugin.Base, {
@@ -41,7 +109,7 @@ YUI.add('addon-rs-config', function(Y, NAME) {
* @param {object} config Configuration object as per Y.Plugin.Base
* @return {nothing}
*/
- initializer: function(config) {
+ initializer: function (config) {
this.appRoot = config.appRoot;
this.mojitoRoot = config.mojitoRoot;
this.afterHostMethod('findResourceVersionByConvention', this.findResourceVersionByConvention, this);
@@ -58,7 +126,7 @@ YUI.add('addon-rs-config', function(Y, NAME) {
* @method getDimensions
* @return {object} the YCB dimensions structure for the app
*/
- getDimensions: function() {
+ getDimensions: function () {
return Y.mojito.util.copy(this._ycbDims);
},
@@ -69,7 +137,7 @@ YUI.add('addon-rs-config', function(Y, NAME) {
* @param {string} fullPath path to JSON or YAML file
* @return {user-defined} contents of file as an object
*/
- readConfigSimple: function(fullPath) {
+ readConfigSimple: function (fullPath) {
var json,
contents;
if (!existsSync(fullPath)) {
@@ -79,6 +147,7 @@ YUI.add('addon-rs-config', function(Y, NAME) {
if (!json) {
try {
contents = libfs.readFileSync(fullPath, 'utf-8');
+ contents = stripCommentsInJSON(contents);
json = JSON.parse(contents);
} catch (e) {
throw new Error('Error parsing JSON file: ' + fullPath);
@@ -111,6 +180,7 @@ YUI.add('addon-rs-config', function(Y, NAME) {
raw = libfs.readFileSync(filePath + extensions[i], 'utf8');
try {
if (i === 2) { // json
+ raw = stripCommentsInJSON(raw);
obj = JSON.parse(raw);
json = true;
} else { // yaml or yml
@@ -140,7 +210,7 @@ YUI.add('addon-rs-config', function(Y, NAME) {
* @return {object} the contextualized configuration
*/
// TODO: async interface
- readConfigYCB: function(fullPath, ctx) {
+ readConfigYCB: function (fullPath, ctx) {
var store = this.get('host'),
cacheKey,
json,
@@ -171,7 +241,7 @@ YUI.add('addon-rs-config', function(Y, NAME) {
* @param {string} mojitType name of mojit to which the resource likely belongs
* @return {object||null} for config file resources, returns metadata signifying that
*/
- findResourceVersionByConvention: function(source, mojitType) {
+ findResourceVersionByConvention: function (source, mojitType) {
var fs = source.fs,
use = false;
@@ -218,7 +288,7 @@ YUI.add('addon-rs-config', function(Y, NAME) {
* @param {string} mojitType name of mojit to which the resource likely belongs
* @return {object||null} for config file resources, returns the resource metadata
*/
- parseResourceVersion: function(source, type, subtype, mojitType) {
+ parseResourceVersion: function (source, type, subtype, mojitType) {
var baseParts,
res;
@@ -253,17 +323,15 @@ YUI.add('addon-rs-config', function(Y, NAME) {
* @method _readYcbDimensions
* @return {array} contents of the dimensions.json file
*/
- _readYcbDimensions: function() {
+ _readYcbDimensions: function () {
var path = libpath.join(this.appRoot, 'dimensions.json');
if (!existsSync(path)) {
path = libpath.join(this.mojitoRoot, 'dimensions.json');
}
return this.readConfigSimple(path);
}
-
-
});
- Y.namespace('mojito.addons.rs');
- Y.mojito.addons.rs.config = RSAddonConfig;
-}, '0.0.1', { requires: ['plugin', 'oop', 'mojito-util']});
+ Y.namespace('mojito.addons.rs').config = RSAddonConfig;
+
+}, '0.0.1', { requires: ['plugin', 'oop', 'mojito-util'] });
Something went wrong with that request. Please try again.