yaml-diff-patch icon indicating copy to clipboard operation
yaml-diff-patch copied to clipboard

Formatting issues

Open joeveiga opened this issue 4 years ago • 6 comments

Test 1 (Extra new lines are removed)

const yaml = `
a: 1



b: 1
`;
const result = yamlOverwrite(yaml, { a: 2, b: 2 });
console.log(result);

Expected

a: 2



b: 2

Actual

a: 2

b: 2

Test 1 (Empty comments are removed sometimes)

const yaml = `
a: 1
#
# some doc
#
b: 1
#
c: 1
`;
const result = yamlOverwrite(yaml, { a: 2, b: 2, c: 2 });
console.log(result);

Expected

a: 2
#
# some doc
#
b: 2
#
c: 2

Actual

a: 2
# some doc
#
b: 2
c: 2

joeveiga avatar Sep 16 '21 14:09 joeveiga

Any formatting issues (and there are a few) of this library is entirely up to the underlying yaml parsers/generator used, which currently is yaml. I've been looking around for alternatives - yaml parsers which provide you with a CST (or a decorated AST) and which provide you with abilities to customize the generated output. This isn't possible today - the only solution would be to re-implement yet another yaml parser, which is a big task.

I think the only solution is finding the appropriate underlying parser able to handle this. Neither yaml or js-yaml seem to allow this unfortunately.

grantila avatar Jan 13 '22 19:01 grantila

FYI, Version 2 of the yaml library (currently 2.0.0-10) actually does support a CST, and using it you can greatly reduce the formatting impact of making changes. In particular, it doesn't reformat anything you haven't actually modified, and a lot of in-place changes won't cause any reformatting either. (I haven't tried insert/delete/rearrange ops with it yet, though, just modifying scalars in place.)

pjeby avatar Jan 16 '22 23:01 pjeby

@pjeby Hi! 2.0.0 has had a few even worse formatting issues, but I didn't check until version -6. -10 seems a lot better, and there's a PR to fix the preservation of indentation per block.

grantila avatar Jan 17 '22 23:01 grantila

The second part of this issue should be fixed now, in 1.2.0. The first one I'm unsure right now..

grantila avatar Jan 17 '22 23:01 grantila

10 seems a lot better, and there's a PR to fix the preservation of indentation per block

Yeah, you're talking about the high-level API; I'm saying that you can also tell it to keep the CST and then make changes to the CST that don't reformat anything except the part you changed. Here's some code that I'm using to edit values in fields named tag or tags or a case-insensitive variation thereof:

https://github.com/pjeby/tag-wrangler/blob/f18f6b92e0b3e9db2144b46cdc92c3b6855cc8a1/src/File.js#L49-L80

It uses the high-level API to find the nodes, then uses CST.setScalarValue() to change the CST, and then writes out the changed file with CST.stringify().

Obviously, it gets more complicated if you want to add/remove/reorg stuff, but there is now an API for working directly with the CST and retaining all formatting of anything you don't change, and keeping most of the whitespace/comments on scalar values you modify in-place. I'd be very interested in using this library for other Obsidian plugins if you ever switch it to use the CST; I got too many AST-related formatting complaints (e.g. https://github.com/pjeby/tag-wrangler/issues/26), which is why I'm now using yaml 2's CST API.

pjeby avatar Jan 17 '22 23:01 pjeby

This looks interesting but far trickier than your example I'm afraid. The setScalarValue seems to only handle strings, from what I understand. I'm going to go through the documentation on the CST to see if this could be used for complex cases too, it would surely be ideal!

grantila avatar Jan 18 '22 07:01 grantila