From 6fa40d98070dd4b5eff679a603f117444c31c716 Mon Sep 17 00:00:00 2001 From: Dina Berry Date: Sun, 10 Mar 2019 05:56:05 -0700 Subject: [PATCH 1/3] #470 fix --- lib/js-yaml/dumper.js | 18 ++++++----------- test/issues/0470.js | 46 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 12 deletions(-) create mode 100644 test/issues/0470.js diff --git a/lib/js-yaml/dumper.js b/lib/js-yaml/dumper.js index 86f34794..356e107a 100644 --- a/lib/js-yaml/dumper.js +++ b/lib/js-yaml/dumper.js @@ -194,14 +194,7 @@ function isPlainSafe(c) { // where nb-char ::= c-printable - b-char - c-byte-order-mark. return isPrintable(c) && c !== 0xFEFF // - c-flow-indicator - && 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; + && c !== CHAR_COLON; } // Simplified test for values allowed as the first character in plain style. @@ -272,7 +265,7 @@ function chooseScalarStyle(string, singleLineOnly, indentPerLevel, lineWidth, te if (!isPrintable(char)) { return STYLE_DOUBLE; } - plain = plain && isPlainSafe(char); + plain = plain && (isPlainSafe(char) && !isWhitespace(string.charCodeAt(i + 1))); } } else { // Case: block styles permitted. @@ -291,12 +284,14 @@ function chooseScalarStyle(string, singleLineOnly, indentPerLevel, lineWidth, te } else if (!isPrintable(char)) { return STYLE_DOUBLE; } - plain = plain && isPlainSafe(char); + if (!isPlainSafe(char)) { + plain = plain && !isWhitespace(string.charCodeAt(i + 1)); + } } // in case the end is missing a \n hasFoldableLine = hasFoldableLine || (shouldTrackWidth && (i - previousLineBreak - 1 > lineWidth && - string[previousLineBreak + 1] !== ' ')); + string[previousLineBreak + 1] !== ' ')); } // Although every style can represent \n without escaping, prefer block styles // for multiline, since they're more readable and they don't add empty lines. @@ -760,7 +755,6 @@ function writeNode(state, level, object, block, compact, iskey) { state.dump = '!<' + state.tag + '> ' + state.dump; } } - return true; } diff --git a/test/issues/0470.js b/test/issues/0470.js new file mode 100644 index 00000000..13d2a6c6 --- /dev/null +++ b/test/issues/0470.js @@ -0,0 +1,46 @@ +'use strict'; + +var assert = require('assert'); +var yaml = require('../..'); + +test('should not unnecessaryly apply quotes', function () { + + var expected = 'url: https://github.com/nodeca/js-yaml\n'; + var actual = yaml.dump( + { + url: 'https://github.com/nodeca/js-yaml' + } + ); + + assert.strictEqual(actual, expected); +}); +test('should not unnecessaryly 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); +}); + +test('should not unnecessaryly apply quotes - space then /\n at end of value', 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); +}); +test('should not unnecessaryly 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' }); + + assert.strictEqual(actual, expected); +}); From 0ab1fb647f438a49c9be1bea64204c11faa249a4 Mon Sep 17 00:00:00 2001 From: Ger Hobbelt Date: Tue, 16 Apr 2019 00:22:44 +0200 Subject: [PATCH 2/3] extended tests for #470, including bugfix for the dumper: now all round trip tests pass. # Conflicts: # test/issues/0470.js --- lib/js-yaml/dumper.js | 16 ++-- test/issues/0470.js | 139 ++++++++++++++++++++++++++----- test/units/dump-scalar-styles.js | 4 +- 3 files changed, 127 insertions(+), 32 deletions(-) diff --git a/lib/js-yaml/dumper.js b/lib/js-yaml/dumper.js index 356e107a..7fc40f58 100644 --- a/lib/js-yaml/dumper.js +++ b/lib/js-yaml/dumper.js @@ -190,11 +190,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. @@ -255,7 +251,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. @@ -265,7 +263,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. @@ -284,9 +282,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 index 13d2a6c6..b3f5a4f3 100644 --- a/test/issues/0470.js +++ b/test/issues/0470.js @@ -6,41 +6,140 @@ var yaml = require('../..'); test('should not unnecessaryly 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); + + var roundTrip = yaml.safeLoad(actual); + assert.deepStrictEqual(roundTrip, input); }); -test('should not unnecessaryly apply quotes - ', function () { - var expected = 'url: https://github.com/nodeca/js-yaml\n'; +test('should not unnecessaryly 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(input); + assert.strictEqual(actual, expected); + + var roundTrip = yaml.safeLoad(actual); + assert.deepStrictEqual(roundTrip, input); +}); + +test('should not unnecessaryly apply quotes - space after colon', function () { - var actual = yaml.dump(obj); + var expected = 'url: \'https: //github.com/nodeca/js-yaml\'\n'; + 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); }); -test('should not unnecessaryly apply quotes - space then /\n at end of value', function () { +/* + check for the removal of these dumper checks to deliver proper output - var expected = 'url: \'https://github.com/nodeca/js-yaml \'\n'; + && 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 obj = {}; - obj['url'] = 'https://github.com/nodeca/js-yaml '; + 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#', - var actual = yaml.dump(obj); + colon1: 'C:123', + colon2: 'C: 123', + colon3: 'C :123', + colon4: 'C123:', + colon5: ':C123', + colon6: ':C123:', - assert.strictEqual(actual, expected); -}); -test('should not unnecessaryly apply quotes - space after colon', function () { + comma1: 'C,123', + comma2: 'C, 123', + comma3: 'C ,123', + comma4: 'C123,', + comma5: ',C123', + comma6: ',C123,', - var expected = 'url: \'https: //github.com/nodeca/js-yaml\'\n'; - var actual = yaml.dump({ url: 'https: //github.com/nodeca/js-yaml' }); + 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 index 49f178a0..36f9ade8 100644 --- 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'); }); }); From 2699d76aabf4214a49649689305e4920f0cde3f6 Mon Sep 17 00:00:00 2001 From: Ger Hobbelt Date: Mon, 15 Apr 2019 23:22:47 +0200 Subject: [PATCH 3/3] spelling correction for #470 test descriptions # Conflicts: # test/issues/0470.js --- test/issues/0470.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/issues/0470.js b/test/issues/0470.js index b3f5a4f3..83a17ca2 100644 --- a/test/issues/0470.js +++ b/test/issues/0470.js @@ -3,7 +3,7 @@ var assert = require('assert'); var yaml = require('../..'); -test('should not unnecessaryly apply quotes', function () { +test('should not unnecessarily apply quotes', function () { var expected = 'url: https://github.com/nodeca/js-yaml\n'; var input = { @@ -17,7 +17,7 @@ test('should not unnecessaryly apply quotes', function () { assert.deepStrictEqual(roundTrip, input); }); -test('should not unnecessaryly apply quotes - space then /\n at end of value', function () { +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 = { @@ -31,7 +31,7 @@ test('should not unnecessaryly apply quotes - space then /\n at end of value', f assert.deepStrictEqual(roundTrip, input); }); -test('should not unnecessaryly apply quotes - space after colon', function () { +test('should not unnecessarily apply quotes - space after colon', function () { var expected = 'url: \'https: //github.com/nodeca/js-yaml\'\n'; var input = {