-
-
Notifications
You must be signed in to change notification settings - Fork 775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dumping add quote to string even it is not necessary #470
Comments
This is the same as #466. Perhaps we can have a new setting to handle the quoting issue? |
@joshuaavalon "unquoting" quality is limited only by authors time. If anyone has time to improve it - PR is welcome. |
@puzrin I'll make a PR this weekend, if no one does it before then. |
@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'; assert.strictEqual(actual, expected); var expected = 'url: https://github.com/nodeca/js-yaml\n'; var obj = {}; 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 = {}; var actual = yaml.dump(obj); assert.strictEqual(actual, expected); var expected = 'url: 'https: //github.com/nodeca/js-yaml'\n'; assert.strictEqual(actual, expected); |
Could you format you message and add proper lang type for highlight? |
…ll round trip tests pass.
…ll round trip tests pass. # Conflicts: # test/issues/0470.js
# Conflicts: # test/issues/0470.js
…e rendered output is ambiguous and hence erroneous YAML.
Having a same problem here. Since only colon and hash characters are limited in flow scalars, is a PR still welcome by now? |
@ZEROPC I would appreciate a fix but but not speaking for the project. Crossing fingers :D |
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: 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. If you just put > require('js-yaml').dump({"CC#": "some data"})
'CC#: some data\n'
> require('js-yaml').load('CC#: some data\n')
{ 'CC#': 'some data' } |
As title.
Test Code
Expected Result
Actual Result
Problem
js-yaml/lib/js-yaml/dumper.js
Line 294 in b6d2609
js-yaml/lib/js-yaml/dumper.js
Lines 192 to 205 in b6d2609
All of these characters ( except
0xFEFF
) are safe inside string. Only colon cannot be followed by space or placed at the end of line.The text was updated successfully, but these errors were encountered: