Skip to content
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

Escape double quotes in string prompts #388

Merged
merged 4 commits into from
Mar 20, 2017
Merged

Escape double quotes in string prompts #388

merged 4 commits into from
Mar 20, 2017

Conversation

riesinger
Copy link

This fixes #382.
It checks if the answer type is string, and if it is, replaces " with \", so it can be parsed as valid JSON.

@posva
Copy link
Member

posva commented Mar 8, 2017

Can you add a test case for this?
You can reuse one of the tests and verify that the generated package json is correct by parsing it

@riesinger
Copy link
Author

riesinger commented Mar 8, 2017 via email

@posva
Copy link
Member

posva commented Mar 8, 2017

Yes, so that the double quotes are escaped correctly

@riesinger
Copy link
Author

riesinger commented Mar 8, 2017 via email

@riesinger
Copy link
Author

I just added a test to check the escaping. I tired to leave the other tests alone and add it seperately... May not be the best test, but it should do the trick 😄

test/e2e/test.js Outdated
const pkg = fs.readFileSync(`${MOCK_TEMPLATE_BUILD_PATH}/package.json`, 'utf8')
try {
var validData = JSON.parse(pkg)
expect(validData.author).to.equal(escapedAuthor)
Copy link
Member

Choose a reason for hiding this comment

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

Can't you just do expect(validData.author).to.equal(escapedAnswers.author) ?

Copy link
Author

Choose a reason for hiding this comment

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

Jup, could definitively do that.... My mistake 😂

Copy link
Author

Choose a reason for hiding this comment

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

Just woke up an hour ago, still not very awake

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

@zigomir
Copy link
Contributor

zigomir commented Mar 9, 2017

LGTM 👍

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

Successfully merging this pull request may close these issues.

3 participants