Comment causes issue with ScalarStyle.FOLDED

[jonasfj]

Example

import 'package:yaml/yaml.dart';
import 'package:yaml_edit/yaml_edit.dart';

void main() {
  final doc = YamlEditor('''
- 0
- 1 # comment
- 2
''');

  final node = wrapAsYamlNode(
    'hello',
    scalarStyle: ScalarStyle.FOLDED,
  );
  doc.update([1], node);

  print(doc);
}

We should have a test case covering this. And ideally more test cases covering similar cases.

And we should fix this bug. Maybe we need to move the comment to the next line.
Or maybe we need to drop the comment in cases like this.

[MikiPAUL]

This issue occurs even with ScalarStyle.LITERAL

[MikiPAUL]

Hi @jonasfj, This issue involves checking if the value that was passed in the update function is of scalar style (FOLDED or LITERAL) and the node to get updated with docs contains a comment, then removing the comment and proceeding with a normal update. Will this solve the issue?

[jonasfj]

Any other ways we can solve this?

Do the same thing happen if I try to insert a map or list in block mode?

Is it possible to allow to comment to remain before/after the text?

[MikiPAUL]

- 1
- >- #comment
   hello
- 2

We could allow comments to remain before the text. To do so, we have to insert comments in replacement string and increase the length to cover the comment. Similar approaches can be used with map and list in block mode.
https://github.com/dart-lang/yaml_edit/blob/c9e82f0cc8b6609493ae891afd4f4901d68afa2c/lib/src/source_edit.dart#L129-L131

[jonasfj]

Allowing the comment before will probably backfire in cases like:

- 1
- 2 # line1
    # line2
    # line3
- 3

[jonasfj]

This could work:

- 1
- >
  new text with 2 space indentation
 # comment with one space indentation
- 3

but it also fail if the comment is like:

- 1
- 2 # This
      # is
        # probabluy
          # not
            # sane
- 3

Then we'll be forced to mess with the whitespacing in some way.

---

Perhaps:

- 1 
#comment
- >-
   hello
- 2

is the most reasonable thing, in the degenerate case that becomes:

- 1
# This
# is
# probabluy
# not
# sane
- >
  hello
- 3

or we could do:

key1: 1
key2: >
  hello
# This
# is
# probabluy
# not
# sane
key3: 3

Just always move the comment out to have zero indentation, so it never messes with anything. We might mangle it a bit, if it contains ascii art that depends on the indentation. But that seems acceptable.

This might be better because we don't modify any of the key encodings.

---

Options I think are possibly good:

• (A) Remove comments when switching from flow-mode to strings in block mode.
• (B) Move the comment to the start of the line, like \n#<comment>.

I think both (A) and (B) are reasonable.

I'm not sure (B) isn't so bad -- even if the mutation gets a bit large.

But I have to ask, if maybe removing the comment (option A), isn't actually the right choice.

Imagine:

keyA: valA
keyB:
  sub1key: value1
  # comment that should probably be removed, if we replace `keyB` with a string
  sub2key: value2
keyC: valC

becomes:

keyA: valA
keyB: >
  String that replaced the object with sub1key and sub2key
keyC: valC

It would be surprising if the comment embedded inside the object that was replaced is preserved. So similarly, in a case like:

keyA: valA
keyB: 3 # best number ever!
keyC: valC

becomes

keyA: valA
keyB: 42
keyC: valC

Well, we could argue that the comment # best number ever! was part of the value replaced, thus, it might be very reasonable to remove it.

---

Sorry, for the long trail of thoughts, I'm starting to conclude that comments after a value that is replaced, should be removed. At-least it should be okay to do when switching from flow-mode to block-mode for strings.

What are the scenarios in which we want to do this?

I imagine it'll also be relevant if we do:

keyA: valA
keyB: {subkey: value} # comment
keyC: valC

into:

keyA: valA
keyB: >
  hello world
keyC: valC

[MikiPAUL]

Yeah, It perfectly makes sense to remove the comments after a value that is replaced. Should we do the same with null content also?

- 0
- # comment
- 2

[jonasfj]

yeah, we probably just should remove the comment in all such cases.