Dumping add quote to string even it is not necessary

As title.

Test Code

const jsYaml = require("js-yaml");
const fs = require("fs");
const yamlStr = jsYaml.dump({ url: "https://github.com/nodeca/js-yaml" });
fs.writeFileSync("foo.yml", yamlStr, { encoding: "utf8" });

Expected Result

url: https://github.com/nodeca/js-yaml

Actual Result

url: 'https://github.com/nodeca/js-yaml'

Problem

js-yaml/lib/js-yaml/dumper.js

Line 294 in b6d2609

plain = plain && isPlainSafe(char);

js-yaml/lib/js-yaml/dumper.js

Lines 192 to 205 in b6d2609

    
           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_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; 
        
           }

All of these characters ( except0xFEFF ) are safe inside string. Only colon cannot be followed by space or placed at the end of line.

This is the same as #466. Perhaps we can have a new setting to handle the quoting issue?

@joshuaavalon

@joshuaavalon "unquoting" quality is limited only by authors time. If anyone has time to improve it - PR is welcome.

@puzrin

@puzrin I'll make a PR this weekend, if no one does it before then.

PR 454 seems related

@puzrin

@puzrin Are these tests all valid/correct?

`test.only('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.only('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.only('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.only('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);
});
`

Could you format you message and add proper lang type for highlight?

spelling correction for nodeca#470 test descriptions

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

spelling correction for nodeca#470 test descriptions

Revert nodeca#470/nodeca#473 for dumper: too many situations where th…

fixups following the nodeca#470/nodeca#473 revert action.

Having a same problem here. Since only colon and hash characters are limited in flow scalars, is a PR still welcome by now?

@ZEROPC

@ZEROPC I would appreciate a fix but but not speaking for the project. Crossing fingers :D

Don't quote literals with `:` except when necessary

Fixed in db3f529 (in dev branch currently, to be released later).

I'm experiencing a related issue but not entirely sure is the same one.

We have some cases where the object is like:
{ "CC#": "some data" }

And right now this is being confused with a comment in the library. Is this fix related or should I create a new issue?

And right now this is being confused with a comment in the library. Is this fix related or should I create a new issue?

This fix is not related, so it's a new issue.

But I'm not entirely sure what you're talking about. # symbol is required to have a space/tab/newline before it to be a comment.

If you just put CC#, it's not parsed as a comment:

> require('js-yaml').dump({"CC#": "some data"})
'CC#: some data\n'

> require('js-yaml').load('CC#: some data\n')
{ 'CC#': 'some data' }

Navigation Menu

Repository navigation

Dumping add quote to string even it is not necessary #470

Test Code

Expected Result

Actual Result

Problem

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	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_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;
	}

Dumping add quote to string even it is not necessary #470

Description

Test Code

Expected Result

Actual Result

Problem

Activity

diberry commented on Feb 26, 2019

puzrin commented on Feb 26, 2019

diberry commented on Feb 26, 2019

diberry commented on Mar 2, 2019

diberry commented on Mar 3, 2019

puzrin commented on Mar 3, 2019

kchen-shanghai commented on Jul 2, 2020

Giszmo commented on Jul 22, 2020

rlidwka commented on Dec 10, 2020

mgiamberardino commented on Dec 16, 2020

rlidwka commented on Dec 25, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions