Google Git
Sign in
dart/tools.git/43110adf^!/.
commit
43110adf2a8ca77005298d68215c99e2374f26b8
[log]
author
Jonas Finnemann Jensen <jonasfj@google.com>
Thu May 02 15:42:46 2024 +0200
committer
GitHub <noreply@github.com>
Thu May 02 15:42:46 2024 +0200
tree
fdefbbc3ca9329999b67f0e711f36bba67b8a47b
parent
5cf17f77d6fa8466a1c0db80d12eb8fec9d55709 [diff]
Fix 55 (dart-lang/yaml_edit#75)

Fix removal of last key from map in block-mode when value is empty.

Fixes dart-lang/yaml_edit#55.

When the value is empty the `SourceSpan` for the `YamlNode` representing
the value in a map points to the colon.

Example:
```yaml
foo:
  bar:
```

The `YamlNode` for `foo.bar` has a value of `null` and starts and ends
at the colon `:` following `bar`. This means that removal might leave
the colon behind, which causes invalid YAML.

We have the same issue when removing `foo.bar` from the following YAML
document:

```yaml
foo:
  baz: true
  bar:
```

However, in this case, we have a hack that ensures we always strip away
the any comments that follows `bar`. We do this by deleting up-to the
next newline. If we apply the same hack when removing `foo.bar` in the
first example, then it works.

One could argue that it works by accident, but it's kind of desired that
trailing comments are removed, when the value they are trailing is
removed.
diff --git a/pkgs/yaml_edit/CHANGELOG.md b/pkgs/yaml_edit/CHANGELOG.md
index 9472abf..422afe1 100644
--- a/pkgs/yaml_edit/CHANGELOG.md
+++ b/pkgs/yaml_edit/CHANGELOG.md

@@ -1,6 +1,8 @@
-## 2.2.1-wip
+## 2.2.1
 
 - Require Dart 3.0
+- Fix removal of last key in blockmap when key has no value
+  ([#55](https://github.com/dart-lang/yaml_edit/issues/55)).
 
 ## 2.2.0
 

diff --git a/pkgs/yaml_edit/lib/src/map_mutations.dart b/pkgs/yaml_edit/lib/src/map_mutations.dart
index 321987f..07a2c54 100644
--- a/pkgs/yaml_edit/lib/src/map_mutations.dart
+++ b/pkgs/yaml_edit/lib/src/map_mutations.dart

@@ -145,6 +145,7 @@
   }
 
   /// +1 accounts for the colon
+  // TODO: What if here is a whitespace following the key, before the colon?
   final start = keyNode.span.end.offset + 1;
   var end = getContentSensitiveEnd(map.nodes[key]!);
 
@@ -175,9 +176,19 @@
   final keySpan = keyNode.span;
   var end = getContentSensitiveEnd(valueNode);
   final yaml = yamlEdit.toString();
+  final lineEnding = getLineEnding(yaml);
 
   if (map.length == 1) {
     final start = map.span.start.offset;
+    final nextNewLine = yaml.indexOf(lineEnding, end);
+    if (nextNewLine != -1) {
+      // Remove everything up to the next newline, this strips comments that
+      // follows on the same line as the value we're removing.
+      // It also ensures we consume colon when [valueNode.value] is `null`
+      // because there is no value (e.g. `key: \n`). Because [valueNode.span] in
+      // such cases point to the colon `:`.
+      end = nextNewLine;
+    }
     return SourceEdit(start, end - start, '{}');
   }
 
@@ -187,16 +198,16 @@
   ///
   /// We do this because we suspect that our users will want the inline
   /// comments to disappear too.
-  final nextNewLine = yaml.indexOf('\n', end);
+  final nextNewLine = yaml.indexOf(lineEnding, end);
   if (nextNewLine != -1) {
-    end = nextNewLine + 1;
+    end = nextNewLine + lineEnding.length;
   }
 
   final nextNode = getNextKeyNode(map, keyNode);
 
   if (start > 0) {
     final lastHyphen = yaml.lastIndexOf('-', start - 1);
-    final lastNewLine = yaml.lastIndexOf('\n', start - 1);
+    final lastNewLine = yaml.lastIndexOf(lineEnding, start - 1);
     if (lastHyphen > lastNewLine) {
       start = lastHyphen + 2;
 
@@ -208,7 +219,7 @@
         end += nextNode.span.start.column;
       }
     } else if (lastNewLine > lastHyphen) {
-      start = lastNewLine + 1;
+      start = lastNewLine + lineEnding.length;
     }
   }
 

diff --git a/pkgs/yaml_edit/pubspec.yaml b/pkgs/yaml_edit/pubspec.yaml
index 272da40..e9b1654 100644
--- a/pkgs/yaml_edit/pubspec.yaml
+++ b/pkgs/yaml_edit/pubspec.yaml

@@ -1,5 +1,5 @@
 name: yaml_edit
-version: 2.2.1-wip
+version: 2.2.1
 description: >-
   A library for YAML manipulation with comment and whitespace preservation.
 repository: https://github.com/dart-lang/yaml_edit

diff --git a/pkgs/yaml_edit/test/testdata/input/issue_55.test b/pkgs/yaml_edit/test/testdata/input/issue_55.test
new file mode 100644
index 0000000..ff1ac90
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/input/issue_55.test

@@ -0,0 +1,11 @@
+TEST FOR ISSUE #55 -- https://github.com/dart-lang/yaml_edit/issues/55
+---
+name: sample
+version: 0.1.0
+environment:
+  sdk: ^3.0.0
+dependencies:
+dev_dependencies:
+  retry:
+---
+ - [remove, ['dev_dependencies', 'retry']]

diff --git a/pkgs/yaml_edit/test/testdata/input/remove_key.test b/pkgs/yaml_edit/test/testdata/input/remove_key.test
new file mode 100644
index 0000000..b28818c
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/input/remove_key.test

@@ -0,0 +1,6 @@
+REMOVE KEY FROM MAP
+---
+foo: true
+bar: false
+---
+- [remove, [bar]]

diff --git a/pkgs/yaml_edit/test/testdata/input/remove_key_with_trailing_comma.test b/pkgs/yaml_edit/test/testdata/input/remove_key_with_trailing_comma.test
new file mode 100644
index 0000000..ce67551
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/input/remove_key_with_trailing_comma.test

@@ -0,0 +1,6 @@
+REMOVE KEY FROM MAP WITH TRAILING COMMA
+---
+foo: true
+bar: false # remove this comment
+---
+- [remove, [bar]]

diff --git a/pkgs/yaml_edit/test/testdata/input/remove_nested_key.test b/pkgs/yaml_edit/test/testdata/input/remove_nested_key.test
new file mode 100644
index 0000000..201caca
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/input/remove_nested_key.test

@@ -0,0 +1,9 @@
+REMOVE NESTED KEY FROM MAP
+---
+A: true
+B:
+  foo: true
+  bar: true
+---
+- [remove, [B, foo]]
+- [remove, [B, bar]]

diff --git a/pkgs/yaml_edit/test/testdata/input/remove_nested_key_with_null.test b/pkgs/yaml_edit/test/testdata/input/remove_nested_key_with_null.test
new file mode 100644
index 0000000..2a66559
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/input/remove_nested_key_with_null.test

@@ -0,0 +1,7 @@
+REMOVE NESTED KEY FROM MAP WITH NULL
+---
+A: true
+B:
+  foo:
+---
+- [remove, [B, foo]]

diff --git a/pkgs/yaml_edit/test/testdata/input/remove_nested_key_with_trailing_comma.test b/pkgs/yaml_edit/test/testdata/input/remove_nested_key_with_trailing_comma.test
new file mode 100644
index 0000000..35ad665
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/input/remove_nested_key_with_trailing_comma.test

@@ -0,0 +1,7 @@
+REMOVE NESTED KEY FROM MAP WITH TRAILING COMMA
+---
+A: true
+B:
+  bar: false # remove this comment
+---
+- [remove, [B, bar]]

diff --git a/pkgs/yaml_edit/test/testdata/output/issue_55.golden b/pkgs/yaml_edit/test/testdata/output/issue_55.golden
new file mode 100644
index 0000000..f752a14
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/output/issue_55.golden

@@ -0,0 +1,15 @@
+name: sample
+version: 0.1.0
+environment:
+  sdk: ^3.0.0
+dependencies:
+dev_dependencies:
+  retry:
+---
+name: sample
+version: 0.1.0
+environment:
+  sdk: ^3.0.0
+dependencies:
+dev_dependencies:
+  {}

diff --git a/pkgs/yaml_edit/test/testdata/output/remove_key.golden b/pkgs/yaml_edit/test/testdata/output/remove_key.golden
new file mode 100644
index 0000000..48b8640
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/output/remove_key.golden

@@ -0,0 +1,4 @@
+foo: true
+bar: false
+---
+foo: true

diff --git a/pkgs/yaml_edit/test/testdata/output/remove_key_with_trailing_comma.golden b/pkgs/yaml_edit/test/testdata/output/remove_key_with_trailing_comma.golden
new file mode 100644
index 0000000..d85be35
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/output/remove_key_with_trailing_comma.golden

@@ -0,0 +1,4 @@
+foo: true
+bar: false # remove this comment
+---
+foo: true

diff --git a/pkgs/yaml_edit/test/testdata/output/remove_nested_key.golden b/pkgs/yaml_edit/test/testdata/output/remove_nested_key.golden
new file mode 100644
index 0000000..011f1c1
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/output/remove_nested_key.golden

@@ -0,0 +1,12 @@
+A: true
+B:
+  foo: true
+  bar: true
+---
+A: true
+B:
+  bar: true
+---
+A: true
+B:
+  {}

diff --git a/pkgs/yaml_edit/test/testdata/output/remove_nested_key_with_null.golden b/pkgs/yaml_edit/test/testdata/output/remove_nested_key_with_null.golden
new file mode 100644
index 0000000..b0e771c
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/output/remove_nested_key_with_null.golden

@@ -0,0 +1,7 @@
+A: true
+B:
+  foo:
+---
+A: true
+B:
+  {}

diff --git a/pkgs/yaml_edit/test/testdata/output/remove_nested_key_with_trailing_comma.golden b/pkgs/yaml_edit/test/testdata/output/remove_nested_key_with_trailing_comma.golden
new file mode 100644
index 0000000..c93c46a
--- /dev/null
+++ b/pkgs/yaml_edit/test/testdata/output/remove_nested_key_with_trailing_comma.golden

@@ -0,0 +1,7 @@
+A: true
+B:
+  bar: false # remove this comment
+---
+A: true
+B:
+  {}