dart-lang / yaml_edit

A library for YAML manipulation with comment and whitespace preservation.
https://pub.dev/packages/yaml_edit
BSD 3-Clause "New" or "Revised" License
27 stars 16 forks source link

the YamlEditor.update method can leave trailing spaces at eol #29

Closed devoncarew closed 1 year ago

devoncarew commented 1 year ago

Unit test repro included; for some cases, the YamlEditor.update method can leave trailing spaces at eol. So below, where we'd expect updates:, we instead get updates:□.

  test('no trailing whitespace', () {
    final editor = YamlEditor('''
updates: null
''');
    editor.update(['updates'], ['foo']);

    expect(editor.toString(), '''
updates:
  - foo
''');
  });
jonasfj commented 1 year ago

Makes sense that this can happen when rewriting from null to list in block mode.

Probably, it can also happen if rewriting from "foo" to list or object in block mode.

Ishad-M-I-M commented 1 year ago

Hi @jonasfj . Can I get this assigned to me

jonasfj commented 1 year ago

sure!

Ishad-M-I-M commented 1 year ago

Hi @jonasfj . Fix to this issue breaks some of the existing testcases as they are written such that to leave a whitespace at eol on update.

4 of existing testcases in update_test.dart fails. Below is an example.

      test('nested (8)', () {
        final doc = YamlEditor('''
a:
b: false
''');
        doc.update(['a'], {'retry': '3.0.1'});

        expect(doc.toString(), equals('''
a: 
  retry: 3.0.1
b: false
'''));
      });

This fails with following message.

Expected: 'a: \n'
            '  retry: 3.0.1\n'
            'b: false\n'
            ''
  Actual: 'a:\n'
            '  retry: 3.0.1\n'
            'b: false\n'
            ''
   Which: is different.
          Expected: a: \n  retry ...
            Actual: a:\n  retry: ...
                      ^
           Differ at offset 2

Should I need to reformat these expected result string by removing whitespace at eol? There are few testcases from other files as well which will fails by this scenario.

jonasfj commented 1 year ago

Should I need to reformat these expected result string by removing whitespace at eol?

Yes -- I think it's entirely possible that test cases also have bugs :D

feinstein commented 1 year ago

Hi, can we get a release with this fix? (Preferably just this fix, not the dart version bump). Thanks!