Skip to content
This repository has been archived by the owner on Apr 28, 2020. It is now read-only.

Hack to handle single quotes #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mcraiha
Copy link
Contributor

@mcraiha mcraiha commented Oct 14, 2018

I assume the modified test case is now better match to https://github.com/json5/json5/blob/master/test/stringify.js since C# doesn't understand single quoted strings

Not my finest work, fixes:
SingleQuotedStringPropertyNamesTest
DoubleQuotedStringPropertyNamesTest

Not my finest work, fixes:
SingleQuotedStringPropertyNamesTest
DoubleQuotedStringPropertyNamesTest
@jordanbtucker jordanbtucker self-assigned this Oct 14, 2018
Copy link
Member

@jordanbtucker jordanbtucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DoubleQuotedStringPropertyNamesTest ensures that double quotes are wrapped around property names that contain single quotes.

In JavaScript JSON5.stringify({'a\'b':1}) should result in {"a'b":1} rather than {'a\'b':1}. The goal is to output the minimum number of escapes possible.

Compare the following statements that stringify strings.

JSON5.stringify(`Jane said, "Don't lie to me!"`)
// 'Jane said, "Don\'t lie to me!"'

JSON5.stringify(`"Bob's wife is Alice's sister's mother," said Joe.`)
// "\"Bob's wife is Alice's sister's mother,\" said Joe."

So, if there are more single quotes than double quotes in a string or property name, then double quotes are used, otherwise, single quotes are used.

See https://github.com/json5/json5/blob/69c4a75d345a58a773148dd9c05ce74e668dc87d/lib/stringify.js#L150-L154

@@ -22,7 +22,7 @@ public void UnquotedPropertyNamesTest()
[TestMethod]
public void SingleQuotedStringPropertyNamesTest()
{
var s = Json5.Stringify(new Json5Object { { "a-b", 1 } });
var s = Json5.Stringify(new Json5Object { { "'a-b'", 1 } });
Copy link
Member

@jordanbtucker jordanbtucker Oct 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of SingleQuotedStringPropertyNamesTest is to ensure that Stringify puts single quotes around property names that are not ES5 Identifiers.

In the original test, the property name was a-b, but your change makes it 'a-b'—a property name that contains single quotes.

If you ran the original test against your hack, it would fail because {'a-b':1} is not equal to {a-b:1}. Your hack actually outputs invalid JSON5.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants