Add "noArrayIndent" option#461
Conversation
| function State(options) { | ||
| this.schema = options['schema'] || DEFAULT_FULL_SCHEMA; | ||
| this.indent = Math.max(1, (options['indent'] || 2)); | ||
| this.indentArrays = (options['indentArrays'] === undefined) ? true : !!options['indentArrays']; |
There was a problem hiding this comment.
Defaulting to true to maintain backwards-compatibility.
| benchmark/implementations/* | ||
| !/benchmark/implementations/current/ | ||
| .idea | ||
| package-lock.json |
There was a problem hiding this comment.
The package lock should either be committed or ignored. Seeing as it isn't committed, I'm ignoring it.
There was a problem hiding this comment.
Don't touch configs if PR is not related
| { | ||
| "name": "js-yaml", | ||
| "version": "3.12.0", | ||
| "version": "3.13.0", |
There was a problem hiding this comment.
Not sure if I'm supposed to version bump or not...
puzrin
left a comment
puzrin
left a comment
There was a problem hiding this comment.
- squash all to single commit pleaze
| benchmark/implementations/* | ||
| !/benchmark/implementations/current/ | ||
| .idea | ||
| package-lock.json |
There was a problem hiding this comment.
Don't touch configs if PR is not related
| function State(options) { | ||
| this.schema = options['schema'] || DEFAULT_FULL_SCHEMA; | ||
| this.indent = Math.max(1, (options['indent'] || 2)); | ||
| this.indentArrays = (typeof options['indentArrays'] === 'undefined') ? true : !!options['indentArrays']; |
There was a problem hiding this comment.
I'd suggest noArrayIndent instead of such logic.
| { | ||
| "name": "js-yaml", | ||
| "version": "3.12.0", | ||
| "version": "3.13.0", |
|
@puzrin I have completed all requested changes, and squashed all the commits down to a single commit as requested. Thanks |
|
FYI: I highly recommend you use "squash merges" functionality in the repository settings, rather than requiring contributors to rewrite history on their feature branches. |
|
Thanks! I know about "squash merges". But it just more convenient for review to to collapse long tail of commits, when only couple of files affected. |
|
@jacob-hd could you take a look at #468 (comment)? Seems indents become broken when top level array contains objects |
=> One more example of bad behaviour. Seems initial PR has not enougth nested samples. |
|
Following on from what @puzrin said about the issue with noArrayIndent, the following also does not work: => vs expected I suspect that the issue might be resulting from an earlier fix by @diberry (namely https://github.com/diberry/js-yaml/commit/18a871cf69435309598393be86006e39abc9d492), where the level should be >= 0, rather than just > 0, but I am not entirely sure. Just trying to provide a pointer :) |
3 participants
Addresses issue #432 by adding a
noArrayIndentoption to optionally not add an extra level of indentation to array elements.When
noArrayIndentoption is set tofalse(or not provided), output is:When
noArrayIndentoption is set totrue, output is:This helps avoid diffs when parsing, modifying, and generating valid yaml that does not use extra indentation for arrays.