extended tests for nodeca#470, including bugfix for the dumper: now a…

…ll round trip tests pass.
GerHobbelt · Apr 15, 2019 · c2215dd · c2215dd
1 parent 54ba1f1
commit c2215dd
Show file tree

Hide file tree

Showing 3 changed files with 127 additions and 34 deletions.
diff --git a/lib/js-yaml/dumper.js b/lib/js-yaml/dumper.js
@@ -201,11 +201,7 @@ function isPrintable(c) {
 
 // Simplified test for values allowed after the first character in plain style.
 function isPlainSafe(c) {
-  // Uses a subset of nb-char - c-flow-indicator - ":" - "#"
-  // where nb-char ::= c-printable - b-char - c-byte-order-mark.
-  return isPrintable(c) && c !== 0xFEFF
-    // - c-flow-indicator
-    && c !== CHAR_COLON;
+  return isPrintable(c) && c !== 0xFEFF;
 }
 
 // Simplified test for values allowed as the first character in plain style.
@@ -269,7 +265,9 @@ function chooseScalarStyle(string, singleLineOnly, indentPerLevel, lineWidth, te
   var shouldTrackWidth = lineWidth !== -1;
   var previousLineBreak = -1; // count the first line correctly
   var plain = isPlainSafeFirst(string.charCodeAt(0))
-          && !isWhitespace(string.charCodeAt(string.length - 1));
+          && !isWhitespace(string.charCodeAt(string.length - 1))
+          && /\s#/.test(string) === false
+          && /:(?:\s|$)/.test(string) === false;
 
   if (singleLineOnly) {
     // Case: no block styles.
@@ -279,7 +277,7 @@ function chooseScalarStyle(string, singleLineOnly, indentPerLevel, lineWidth, te
       if (!isPrintable(char)) {
         return STYLE_DOUBLE;
       }
-      plain = plain && (isPlainSafe(char) && !isWhitespace(string.charCodeAt(i + 1)));
+      plain = plain && isPlainSafe(char);
     }
   } else {
     // Case: block styles permitted.
@@ -298,9 +296,7 @@ function chooseScalarStyle(string, singleLineOnly, indentPerLevel, lineWidth, te
       } else if (!isPrintable(char)) {
         return STYLE_DOUBLE;
       }
-      if (!isPlainSafe(char)) {
-        plain = plain && !isWhitespace(string.charCodeAt(i + 1));
-      }
+      plain = plain && isPlainSafe(char);
     }
     // in case the end is missing a \n
     hasFoldableLine = hasFoldableLine || (shouldTrackWidth &&

diff --git a/test/issues/0470.js b/test/issues/0470.js
@@ -6,43 +6,140 @@ var yaml = require('../..');
 test('should not unnecessarily apply quotes', function () {
 
   var expected = 'url: https://github.com/nodeca/js-yaml\n';
-  var actual = yaml.dump(
-    {
-      url: 'https://github.com/nodeca/js-yaml'
-    }
-  );
+  var input = {
+    url: 'https://github.com/nodeca/js-yaml'
+  };
 
+  var actual = yaml.dump(input);
   assert.strictEqual(actual, expected);
-});
-
-test('should not unnecessarily apply quotes - ', function () {
-
-  var expected = 'url: https://github.com/nodeca/js-yaml\n';
 
-  var obj = {};
-  obj['url'] = 'https://github.com/nodeca/js-yaml';
-
-  var actual = yaml.dump(obj);
-
-  assert.strictEqual(actual, expected);
+  var roundTrip = yaml.safeLoad(actual);
+  assert.deepStrictEqual(roundTrip, input);
 });
 
 test('should not unnecessarily apply quotes - space then /\n at end of value', function () {
 
   var expected = 'url: \'https://github.com/nodeca/js-yaml \'\n';
+  var input = {
+    url: 'https://github.com/nodeca/js-yaml '
+  };
 
-  var obj = {};
-  obj['url'] = 'https://github.com/nodeca/js-yaml ';
-
-  var actual = yaml.dump(obj);
-
+  var actual = yaml.dump(input);
   assert.strictEqual(actual, expected);
+
+  var roundTrip = yaml.safeLoad(actual);
+  assert.deepStrictEqual(roundTrip, input);
 });
 
 test('should not unnecessarily apply quotes - space after colon', function () {
 
   var expected = 'url: \'https: //github.com/nodeca/js-yaml\'\n';
-  var actual = yaml.dump({ url: 'https: //github.com/nodeca/js-yaml' });
+  var input = {
+    url: 'https: //github.com/nodeca/js-yaml'
+  };
 
+  var actual = yaml.dump(input);
   assert.strictEqual(actual, expected);
+
+  var roundTrip = yaml.safeLoad(actual);
+  assert.deepStrictEqual(roundTrip, input);
+});
+
+/*
+  check for the removal of these dumper checks to deliver proper output
+
+    && c !== CHAR_COMMA
+    && c !== CHAR_LEFT_SQUARE_BRACKET
+    && c !== CHAR_RIGHT_SQUARE_BRACKET
+    && c !== CHAR_LEFT_CURLY_BRACKET
+    && c !== CHAR_RIGHT_CURLY_BRACKET
+    // - ":" - "#"
+    && c !== CHAR_COLON
+    && c !== CHAR_SHARP;
+ */
+test('should not unnecessarily apply quotes - : colon, # sharp, comma and []{}', function () {
+
+  var expected = [
+    'sharp1: C#123',
+    'sharp2: C# 123',
+    "sharp3: 'C #123'",
+    'sharp4: C123#',
+    "sharp5: '#C123'",
+    "sharp6: '#C123#'",
+    'colon1: C:123',
+    "colon2: 'C: 123'",
+    'colon3: C :123',
+    "colon4: 'C123:'",
+    "colon5: ':C123'",
+    "colon6: ':C123:'",
+    'comma1: C,123',
+    'comma2: C, 123',
+    'comma3: C ,123',
+    'comma4: C123,',
+    "comma5: ',C123'",
+    "comma6: ',C123,'",
+    'sqbr1: C[1]23',
+    'sqbr2: C [1]23',
+    "sqbr3: '[C] 123'",
+    'sqbr4: C12[3]',
+    "sqbr5: '[C]123'",
+    "sqbr6: '[C]12[3]'",
+    "sqbr7: '[]C123[]'",
+    'sqbr8: C [123]',
+    'cbr1: C{1}23',
+    'cbr2: C {1}23',
+    "cbr3: '{C} 123'",
+    'cbr4: C12{3}',
+    "cbr5: '{C}123'",
+    "cbr6: '{C}12{3}'",
+    "cbr7: '{}C123{}'",
+    'cbr8: C {123}',
+    ''
+  ].join('\n');
+  var input = {
+    sharp1: 'C#123',
+    sharp2: 'C# 123',
+    sharp3: 'C #123',
+    sharp4: 'C123#',
+    sharp5: '#C123',
+    sharp6: '#C123#',
+
+    colon1: 'C:123',
+    colon2: 'C: 123',
+    colon3: 'C :123',
+    colon4: 'C123:',
+    colon5: ':C123',
+    colon6: ':C123:',
+
+    comma1: 'C,123',
+    comma2: 'C, 123',
+    comma3: 'C ,123',
+    comma4: 'C123,',
+    comma5: ',C123',
+    comma6: ',C123,',
+
+    sqbr1: 'C[1]23',
+    sqbr2: 'C [1]23',
+    sqbr3: '[C] 123',
+    sqbr4: 'C12[3]',
+    sqbr5: '[C]123',
+    sqbr6: '[C]12[3]',
+    sqbr7: '[]C123[]',
+    sqbr8: 'C [123]',
+
+    cbr1: 'C{1}23',
+    cbr2: 'C {1}23',
+    cbr3: '{C} 123',
+    cbr4: 'C12{3}',
+    cbr5: '{C}123',
+    cbr6: '{C}12{3}',
+    cbr7: '{}C123{}',
+    cbr8: 'C {123}'
+  };
+
+  var actual = yaml.dump(input);
+  assert.strictEqual(actual, expected);
+
+  var roundTrip = yaml.safeLoad(actual);
+  assert.deepStrictEqual(roundTrip, input);
 });
diff --git a/test/units/dump-scalar-styles.js b/test/units/dump-scalar-styles.js
@@ -39,9 +39,9 @@ suite('Scalar style dump:', function () {
 
     test('disallows flow indicators inside flow collections', function () {
       assert.strictEqual(yaml.safeDump({ quote: 'mispell [sic]' }, { flowLevel: 0 }),
-        "{quote: 'mispell [sic]'}\n");
+        '{quote: mispell [sic]}\n');
       assert.strictEqual(yaml.safeDump({ key: 'no commas, either' }, { flowLevel: 0 }),
-        "{key: 'no commas, either'}\n");
+        '{key: no commas, either}\n');
     });
   });