Conversation
stringify(data, { comments: true })
save comments to dictionary object upon parsing,
add the option to save when stringifying data
lukekarrys
left a comment
lukekarrys
left a comment
There was a problem hiding this comment.
Thanks for the contribution! I gave it a quick review on mobile. Let me know what you think of the suggestions.
| * Whether to save the comments. | ||
| */ | ||
|
|
||
| comments: false , |
There was a problem hiding this comment.
| comments: false , | |
| comments: false, |
| } | ||
| p = out[section] = out[section] || Object.create(null) | ||
| if (lineCommentArray.length > 0) { | ||
| commentsDictionary[section] = lineCommentArray.join('\n') + '\n' |
There was a problem hiding this comment.
I think this should use platform eol also.
| } else { | ||
| p[key] = value | ||
| if (lineCommentArray.length > 0) { | ||
| commentsDictionary[key] = lineCommentArray.join('\n') + '\n' |
| @@ -1,4 +1,5 @@ | |||
| const { hasOwnProperty } = Object.prototype | |||
| const commentsDictionary = {} | |||
There was a problem hiding this comment.
I don't think this will work to have as a top level dictionary shared across invocations.
If i'm reading it right, if two documents are parsed and they have keys that collide, the comments could end up in the wrong document when stringified in a different order.
|
|
||
| for (const line of lines) { | ||
| if (!line || line.match(/^\s*[;#]/) || line.match(/^\s*$/)) { | ||
| if (line && line.match(commentsRegEx)) { |
There was a problem hiding this comment.
This if block does not need to be nested within its parent. I think it would be better to have a block that checks for empty lines and another that checks for comments.
| const s = i.stringify(d, { comments: true }) | ||
| t.matchSnapshot(s) | ||
| t.end() | ||
| }) |
There was a problem hiding this comment.
I don't think this test case needs to be its own file. It can be tested within the current foo test file.
|
Closing in favor of #247 |
2 participants
support option to save comments when stringifying
#32
#83