Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jonasfj authored May 2, 2024
1 parent 5cf17f7 commit 43110ad
Show file tree
Hide file tree
Showing 15 changed files with 114 additions and 6 deletions.
4 changes: 3 additions & 1 deletion pkgs/yaml_edit/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
19 changes: 15 additions & 4 deletions pkgs/yaml_edit/lib/src/map_mutations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ SourceEdit _replaceInBlockMap(
}

/// +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]!);

Expand Down Expand Up @@ -175,9 +176,19 @@ SourceEdit _removeFromBlockMap(
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, '{}');
}

Expand All @@ -187,16 +198,16 @@ SourceEdit _removeFromBlockMap(
///
/// 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;

Expand All @@ -208,7 +219,7 @@ SourceEdit _removeFromBlockMap(
end += nextNode.span.start.column;
}
} else if (lastNewLine > lastHyphen) {
start = lastNewLine + 1;
start = lastNewLine + lineEnding.length;
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkgs/yaml_edit/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
11 changes: 11 additions & 0 deletions pkgs/yaml_edit/test/testdata/input/issue_55.test
Original file line number Diff line number Diff line change
@@ -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']]
6 changes: 6 additions & 0 deletions pkgs/yaml_edit/test/testdata/input/remove_key.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
REMOVE KEY FROM MAP
---
foo: true
bar: false
---
- [remove, [bar]]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
REMOVE KEY FROM MAP WITH TRAILING COMMA
---
foo: true
bar: false # remove this comment
---
- [remove, [bar]]
9 changes: 9 additions & 0 deletions pkgs/yaml_edit/test/testdata/input/remove_nested_key.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
REMOVE NESTED KEY FROM MAP
---
A: true
B:
foo: true
bar: true
---
- [remove, [B, foo]]
- [remove, [B, bar]]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
REMOVE NESTED KEY FROM MAP WITH NULL
---
A: true
B:
foo:
---
- [remove, [B, foo]]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
REMOVE NESTED KEY FROM MAP WITH TRAILING COMMA
---
A: true
B:
bar: false # remove this comment
---
- [remove, [B, bar]]
15 changes: 15 additions & 0 deletions pkgs/yaml_edit/test/testdata/output/issue_55.golden
Original file line number Diff line number Diff line change
@@ -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:
{}
4 changes: 4 additions & 0 deletions pkgs/yaml_edit/test/testdata/output/remove_key.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
foo: true
bar: false
---
foo: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
foo: true
bar: false # remove this comment
---
foo: true
12 changes: 12 additions & 0 deletions pkgs/yaml_edit/test/testdata/output/remove_nested_key.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
A: true
B:
foo: true
bar: true
---
A: true
B:
bar: true
---
A: true
B:
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
A: true
B:
foo:
---
A: true
B:
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
A: true
B:
bar: false # remove this comment
---
A: true
B:
{}

0 comments on commit 43110ad

Please sign in to comment.