Fix issue with linters that prefer single quotes #57
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Activity: atticoos/issues/12 I ran into [this issue](atticoos/issues/12) as well where linters (Sonarlint in my case) take exception to the double-quotes output. I've changed the template and test files to use single-quotes and am using .replace on the template.constant.value to replace double-quotes with single-quotes. Tests pass.
| @@ -104,7 +104,7 @@ function gulpNgConfig (moduleName, configuration) { | |||
| _.each(jsonObj, function (value, key) { | |||
| constants.push({ | |||
| name: key, | |||
| value: JSON.stringify(value, null, spaces) | |||
| value: JSON.stringify(value, null, spaces).replace(/"/g, '\'') | |||
There was a problem hiding this comment.
What if some of the data contains quotes?
For example:
var input = {
text: 'The person said "Hello" loudly'
}
JSON.stringify(input)
// '{"text":"The person said \"Hello\" loudly"}'
JSON.stringify(input).replace(/"/g, '\'')
// '{\'text\':\'The person said \'Hello\' loudly\'}'That would feel like unexpected behavior to modify the contents
There was a problem hiding this comment.
While this is entirely valid and reasonable safety ought to be a concern, this seems a bit contrived for a "config" module. It's easy enough to fix though, for those edge cases, so I will account for this.
There was a problem hiding this comment.
Totally - however it is a third party dependency for everyone - so we can't manipulate any inputs we don't control or it can lead to unexpected behavior.
Activity: atticoos/pull/57#issuecomment-245965088 To ensure that edge cases are covered with regards to escaped double quotes, I've added some checks to replace those with doubles while replacing the rest with singles.
Activity: atticoos/pull/57#issuecomment-245965088 To preserve backwards compatibility, the single-quote feature has been added as an option. Fully tested, tests pass.
|
Fully tested, singleQuotes as a feature, let me know your thoughts. |
The README has been update to include the new singleQuote feature.
|
Any feedback? |
|
@ajwhite Any idea when this can get merged and released? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Activity: /issues/12
I ran into this issue as well where linters (Sonarlint in my case) take
exception to the double-quotes output. I've changed the template and
test files to use single-quotes and am using .replace on the
template.constant.value to replace double-quotes with single-quotes.
Tests pass.