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

feat: stringify with comments #246

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,13 @@ stringify(object,{
* Some parsers treat duplicate names by themselves as arrays
*/

bracketedArray : true
bracketedArray : true,

/**
* Whether to save the comments.
*/

comments: false ,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
comments: false ,
comments: false,

})
```

Expand Down
25 changes: 24 additions & 1 deletion lib/ini.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const { hasOwnProperty } = Object.prototype
const commentsDictionary = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

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.


const encode = (obj, opt = {}) => {
if (typeof opt === 'string') {
Expand All @@ -12,6 +13,7 @@ const encode = (obj, opt = {}) => {
/* istanbul ignore next */
opt.platform = opt.platform || (typeof process !== 'undefined' && process.platform)
opt.bracketedArray = opt.bracketedArray !== false
opt.comments = opt.comments === true

/* istanbul ignore next */
const eol = opt.platform === 'win32' ? '\r\n' : '\n'
Expand Down Expand Up @@ -52,12 +54,19 @@ const encode = (obj, opt = {}) => {
} else if (val && typeof val === 'object') {
children.push(k)
} else {
if (opt.comments && hasOwnProperty.call(commentsDictionary, k)) {
out += commentsDictionary[k]
}
out += safe(k).padEnd(padToChars, ' ') + separator + safe(val) + eol
}
}

if (opt.section && out.length) {
out = '[' + safe(opt.section) + ']' + (opt.newline ? eol + eol : eol) + out
let sectionComments = ''
if (opt.comments && hasOwnProperty.call(commentsDictionary, opt.section)) {
sectionComments = commentsDictionary[opt.section]
}
out = sectionComments + '[' + safe(opt.section) + ']' + (opt.newline ? eol + eol : eol) + out
}

for (const k of children) {
Expand Down Expand Up @@ -108,13 +117,19 @@ const decode = (str, opt = {}) => {
const out = Object.create(null)
let p = out
let section = null
let lineCommentArray = []
const commentsRegEx = /^[#;]{1,}.*$/
// section |key = value
const re = /^\[([^\]]*)\]\s*$|^([^=]+)(=(.*))?$/i
const lines = str.split(/[\r\n]+/g)
const duplicates = {}

for (const line of lines) {
if (!line || line.match(/^\s*[;#]/) || line.match(/^\s*$/)) {
if (line && line.match(commentsRegEx)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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 commentsMatch = line.match(commentsRegEx)[0]
lineCommentArray.push(commentsMatch)
}
continue
}
const match = line.match(re)
Expand All @@ -130,6 +145,10 @@ const decode = (str, opt = {}) => {
continue
}
p = out[section] = out[section] || Object.create(null)
if (lineCommentArray.length > 0) {
commentsDictionary[section] = lineCommentArray.join('\n') + '\n'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should use platform eol also.

lineCommentArray = []
}
continue
}
const keyRaw = unsafe(match[2])
Expand Down Expand Up @@ -165,6 +184,10 @@ const decode = (str, opt = {}) => {
p[key].push(value)
} else {
p[key] = value
if (lineCommentArray.length > 0) {
commentsDictionary[key] = lineCommentArray.join('\n') + '\n'
Copy link
Contributor

Choose a reason for hiding this comment

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

this one too

lineCommentArray = []
}
}
}

Expand Down
79 changes: 79 additions & 0 deletions tap-snapshots/test/stringify-with-comments.js.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/* IMPORTANT
* This snapshot file is auto-generated, but designed for humans.
* It should be checked into source control and tracked carefully.
* Re-generate by setting TAP_SNAPSHOT=1 and running tests.
* Make sure to inspect the output below. Do not ignore changes!
*/
'use strict'
exports[`test/stringify-with-comments.js TAP stringify with comments > must match snapshot 1`] = `
o=p
a with spaces=b c
; wrap in quotes to JSON-decode and preserve spaces
" xa n p "="\\"\\r\\nyoyoyo\\r\\r\\n"
; wrap in quotes to get a key with a bracket, not a section.
"[disturbing]"=hey you never know
; Test single quotes
s=something
; Test mixing quotes
s1="something'
; Test double quotes
s2=something else
; Test blank value
s3=
; Test value with only spaces
s4=
; Test quoted value with only spaces
s5=" "
; Test quoted value with leading and trailing spaces
s6=" a "
; Test no equal sign
s7=true
; Test bool(true)
true=true
; Test bool(false)
false=false
; Test null
null=null
; Test undefined
undefined=undefined
zr[]=deedee
ar[]=one
ar[]=three
ar[]=this is included
; Test arrays
; This should be included in the array
; Test resetting of a value (and not turn it into an array)
br=warm
eq="eq=eq"

; a section
[a]
av=a val
e={ o: p, a: { av: a val, b: { c: { e: "this [value]" } } } }
j="\\"{ o: \\"p\\", a: { av: \\"a val\\", b: { c: { e: \\"this [value]\\" } } } }\\""
"[]"=a square?
cr[]=four
cr[]=eight

; nested child without middle parent
; should create otherwise-empty a.b
[a.b.c]
e=1
j=2

; dots in the section name should be literally interpreted
[x\\.y\\.z]
x.y.z=xyz

[x\\.y\\.z.a\\.b\\.c]
; nested child without middle parent
; should create otherwise-empty a.b
a.b.c=abc
; this next one is not a comment! it's escaped!
nocomment=this\\; this is not a comment
# Support the use of the number sign (#) as an alternative to the semicolon for indicating comments.
# http://en.wikipedia.org/wiki/INI_file#Comments
# this next one is not a comment! it's escaped!
noHashComment=this\\# this is not a comment

`
16 changes: 16 additions & 0 deletions test/stringify-with-comments.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const i = require('../')
const tap = require('tap')
const test = tap.test
const fs = require('fs')
const path = require('path')
const fixture = path.resolve(__dirname, './fixtures/foo.ini')
const data = fs.readFileSync(fixture, 'utf8')

tap.cleanSnapshot = s => s.replace(/\r\n/g, '\n')

test('stringify with comments', function (t) {
const d = i.parse(data)
const s = i.stringify(d, { comments: true })
t.matchSnapshot(s)
t.end()
})
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this test case needs to be its own file. It can be tested within the current foo test file.

Loading