Skip to content
This repository has been archived by the owner on Jun 27, 2019. It is now read-only.

zapier convert: Escape special chars in field help text #182

Merged
merged 1 commit into from
Nov 14, 2017

Conversation

eliangcs
Copy link
Member

  • Fix redundant slashes when escaping line breaks
  • Escape single quotes (')

* Fix redundant slashes when escaping line breaks
* Escape single quotes (')
Copy link
Contributor

@BrunoBernardino BrunoBernardino left a comment

Choose a reason for hiding this comment

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

This looks great, I only have a question before approving.

const field = s2js(string);
field.helpText.should.eql('line 1\\nline 2\\nline 3\\n');
field.helpText.should.eql('line 1\nline 2\nline 3\n');
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this mean the converted helpText will keep the newlines (which will be a problem when we print them in the template)?

Copy link
Member Author

@eliangcs eliangcs Nov 14, 2017

Choose a reason for hiding this comment

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

That's what I thought when I wrote 'line 1\\nline 2\\nline 3\\n' originally in this test. But since we eval string with s2js to an object, the escaped newlines in the template are unescaped again.

For example:

> eval("({helpText: 'line 1\\nline 2'})");
{ helpText: 'line 1\nline 2' }

Try zapier convert an app that uses multi-lines help text and you'll see how this PR fixes the issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see how that makes sense.

Copy link
Contributor

@BrunoBernardino BrunoBernardino left a comment

Choose a reason for hiding this comment

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

Simple, neat, and with tests!

@eliangcs eliangcs merged commit 8bc0096 into master Nov 14, 2017
@eliangcs eliangcs deleted the escape-special-chars branch November 14, 2017 14:04
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.

None yet

2 participants