Skip to content

Commit 01a4b9d

Browse files
committed
Prevent YamlEditor.remove from affecting existing block scalars
* Refactor `_removeFromBlockList` and `_removeFromBlockMap` and remove dangling line breaks which may affect how block scalars are intepreted by the `package: yaml`. * Prefer obtaining an entry in one pass from `YamlMap`. * Add tests
1 parent c423dbe commit 01a4b9d

File tree

5 files changed

+470
-127
lines changed

5 files changed

+470
-127
lines changed

pkgs/yaml_edit/lib/src/equality.dart

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,20 @@ YamlNode getKeyNode(YamlMap map, Object? key) {
9191
return map.nodes.keys.firstWhere((node) => deepEquals(node, key)) as YamlNode;
9292
}
9393

94+
/// Returns the entry associated with a [mapKey] and its index in the [map].
95+
({int index, YamlNode keyNode, YamlNode valueNode}) getYamlMapEntry(
96+
YamlMap map,
97+
Object? mapKey,
98+
) {
99+
for (final (index, MapEntry(:key, :value)) in map.nodes.entries.indexed) {
100+
if (deepEquals(key, mapKey)) {
101+
return (index: index, keyNode: key, valueNode: value);
102+
}
103+
}
104+
105+
throw YamlException('$mapKey not found in map', map.span);
106+
}
107+
94108
/// Returns the [YamlNode] after the [YamlNode] corresponding to the provided
95109
/// [key].
96110
YamlNode? getNextKeyNode(YamlMap map, Object? key) {

pkgs/yaml_edit/lib/src/list_mutations.dart

Lines changed: 33 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -306,72 +306,43 @@ SourceEdit _insertInFlowList(
306306
/// [index] should be non-negative and less than or equal to `list.length`.
307307
SourceEdit _removeFromBlockList(
308308
YamlEditor yamlEdit, YamlList list, YamlNode nodeToRemove, int index) {
309-
RangeError.checkValueInInterval(index, 0, list.length - 1);
310-
311-
var end = getContentSensitiveEnd(nodeToRemove);
312-
313-
/// If we are removing the last element in a block list, convert it into a
314-
/// flow empty list.
315-
if (list.length == 1) {
316-
final start = list.span.start.offset;
317-
318-
return SourceEdit(start, end - start, '[]');
319-
}
309+
final listSize = list.length;
310+
RangeError.checkValueInInterval(index, 0, listSize - 1);
320311

321312
final yaml = yamlEdit.toString();
322313
final span = nodeToRemove.span;
323314

324-
/// Adjust the end to clear the new line after the end too.
325-
///
326-
/// We do this because we suspect that our users will want the inline
327-
/// comments to disappear too.
328-
final nextNewLine = yaml.indexOf('\n', end);
329-
if (nextNewLine != -1) {
330-
end = nextNewLine + 1;
331-
}
332-
333-
/// If the value is empty
334-
if (span.length == 0) {
335-
var start = span.start.offset;
336-
return SourceEdit(start, end - start, '');
337-
}
338-
339-
/// -1 accounts for the fact that the content can start with a dash
340-
var start = yaml.lastIndexOf('-', span.start.offset - 1);
341-
342-
/// Check if there is a `-` before the node
343-
if (start > 0) {
344-
final lastHyphen = yaml.lastIndexOf('-', start - 1);
345-
final lastNewLine = yaml.lastIndexOf('\n', start - 1);
346-
if (lastHyphen > lastNewLine) {
347-
start = lastHyphen + 2;
348-
349-
/// If there is a `-` before the node, we need to check if we have
350-
/// to update the indentation of the next node.
351-
if (index < list.length - 1) {
352-
/// Since [end] is currently set to the next new line after the current
353-
/// node, check if we see a possible comment first, or a hyphen first.
354-
/// Note that no actual content can appear here.
355-
///
356-
/// We check this way because the start of a span in a block list is
357-
/// the start of its value, and checking from the back leaves us
358-
/// easily confused if there are comments that have dashes in them.
359-
final nextHash = yaml.indexOf('#', end);
360-
final nextHyphen = yaml.indexOf('-', end);
361-
final nextNewLine = yaml.indexOf('\n', end);
362-
363-
/// If [end] is on the same line as the hyphen of the next node
364-
if ((nextHash == -1 || nextHyphen < nextHash) &&
365-
nextHyphen < nextNewLine) {
366-
end = nextHyphen;
367-
}
368-
}
369-
} else if (lastNewLine > lastHyphen) {
370-
start = lastNewLine + 1;
371-
}
372-
}
373-
374-
return SourceEdit(start, end - start, '');
315+
return removeBlockCollectionEntry(
316+
yaml,
317+
blockCollection: list,
318+
isFirstEntry: index == 0,
319+
isSingleEntry: listSize == 1,
320+
isLastEntry: index >= listSize - 1,
321+
nodeToRemoveOffset: (
322+
start: span.length == 0
323+
? span.start.offset
324+
: yaml.lastIndexOf('-', span.start.offset - 1),
325+
end: getContentSensitiveEnd(nodeToRemove)
326+
),
327+
lineEnding: getLineEnding(yaml),
328+
nextBlockNodeInfo: () {
329+
final nextNode = list.nodes[index + 1];
330+
final nextNodeSpan = nextNode.span;
331+
final offset = nextNodeSpan.start.offset;
332+
333+
final hyphenOffset = yaml.lastIndexOf(
334+
'-',
335+
span.length == 0 ? offset : offset - 1,
336+
);
337+
338+
final nearestLineEnding = yaml.lastIndexOf('\n', hyphenOffset);
339+
340+
return (
341+
nearestLineEnding: nearestLineEnding,
342+
nextNodeColStart: hyphenOffset - (nearestLineEnding + 1),
343+
);
344+
},
345+
);
375346
}
376347

377348
/// Returns a [SourceEdit] describing the change to be made on [yamlEdit] to

pkgs/yaml_edit/lib/src/map_mutations.dart

Lines changed: 36 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,11 @@ SourceEdit updateInMap(
3636
/// removing the element at [key] when re-parsed.
3737
SourceEdit removeInMap(YamlEditor yamlEdit, YamlMap map, Object? key) {
3838
assert(containsKey(map, key));
39-
final keyNode = getKeyNode(map, key);
40-
final valueNode = map.nodes[keyNode]!;
4139

4240
if (map.style == CollectionStyle.FLOW) {
43-
return _removeFromFlowMap(yamlEdit, map, keyNode, valueNode);
41+
return _removeFromFlowMap(yamlEdit, map, key);
4442
} else {
45-
return _removeFromBlockMap(yamlEdit, map, keyNode, valueNode);
43+
return _removeFromBlockMap(yamlEdit, map, key);
4644
}
4745
}
4846

@@ -169,74 +167,47 @@ SourceEdit _replaceInFlowMap(
169167
}
170168

171169
/// Performs the string operation on [yamlEdit] to achieve the effect of
172-
/// removing the [keyNode] from the map, bearing in mind that this is a block
170+
/// removing the [key] from the map, bearing in mind that this is a block
173171
/// map.
174-
SourceEdit _removeFromBlockMap(
175-
YamlEditor yamlEdit, YamlMap map, YamlNode keyNode, YamlNode valueNode) {
176-
final keySpan = keyNode.span;
177-
var end = getContentSensitiveEnd(valueNode);
172+
SourceEdit _removeFromBlockMap(YamlEditor yamlEdit, YamlMap map, Object? key) {
173+
final (index: entryIndex, :keyNode, :valueNode) = getYamlMapEntry(map, key);
178174
final yaml = yamlEdit.toString();
179-
final lineEnding = getLineEnding(yaml);
180-
181-
if (map.length == 1) {
182-
final start = map.span.start.offset;
183-
final nextNewLine = yaml.indexOf(lineEnding, end);
184-
if (nextNewLine != -1) {
185-
// Remove everything up to the next newline, this strips comments that
186-
// follows on the same line as the value we're removing.
187-
// It also ensures we consume colon when [valueNode.value] is `null`
188-
// because there is no value (e.g. `key: \n`). Because [valueNode.span] in
189-
// such cases point to the colon `:`.
190-
end = nextNewLine;
191-
} else {
192-
// Remove everything until the end of the document, if there is no newline
193-
end = yaml.length;
194-
}
195-
return SourceEdit(start, end - start, '{}');
196-
}
197-
198-
var start = keySpan.start.offset;
199-
200-
/// Adjust the end to clear the new line after the end too.
201-
///
202-
/// We do this because we suspect that our users will want the inline
203-
/// comments to disappear too.
204-
final nextNewLine = yaml.indexOf(lineEnding, end);
205-
if (nextNewLine != -1) {
206-
end = nextNewLine + lineEnding.length;
207-
} else {
208-
// Remove everything until the end of the document, if there is no newline
209-
end = yaml.length;
210-
}
211-
212-
final nextNode = getNextKeyNode(map, keyNode);
213-
214-
if (start > 0) {
215-
final lastHyphen = yaml.lastIndexOf('-', start - 1);
216-
final lastNewLine = yaml.lastIndexOf(lineEnding, start - 1);
217-
if (lastHyphen > lastNewLine) {
218-
start = lastHyphen + 2;
219-
220-
/// If there is a `-` before the node, and the end is on the same line
221-
/// as the next node, we need to add the necessary offset to the end to
222-
/// make sure the next node has the correct indentation.
223-
if (nextNode != null &&
224-
nextNode.span.start.offset - end <= nextNode.span.start.column) {
225-
end += nextNode.span.start.column;
226-
}
227-
} else if (lastNewLine > lastHyphen) {
228-
start = lastNewLine + lineEnding.length;
229-
}
230-
}
175+
final mapSize = map.length;
176+
final keySpan = keyNode.span;
231177

232-
return SourceEdit(start, end - start, '');
178+
return removeBlockCollectionEntry(
179+
yaml,
180+
blockCollection: map,
181+
isFirstEntry: entryIndex == 0,
182+
isSingleEntry: mapSize == 1,
183+
isLastEntry: entryIndex >= mapSize - 1,
184+
nodeToRemoveOffset: (
185+
start: keySpan.start.offset,
186+
end: valueNode.span.length == 0
187+
? keySpan.end.offset + 2 // Null value have no span. Skip ":".
188+
: getContentSensitiveEnd(valueNode),
189+
),
190+
lineEnding: getLineEnding(yaml),
191+
192+
// Only called when the next node is present. Never before.
193+
nextBlockNodeInfo: () {
194+
final nextKeyNode = map.nodes.keys.elementAt(entryIndex + 1) as YamlNode;
195+
final nextKeySpan = nextKeyNode.span.start;
196+
197+
return (
198+
nearestLineEnding: yaml.lastIndexOf('\n', nextKeySpan.offset),
199+
nextNodeColStart: nextKeySpan.column
200+
);
201+
},
202+
);
233203
}
234204

235205
/// Performs the string operation on [yamlEdit] to achieve the effect of
236-
/// removing the [keyNode] from the map, bearing in mind that this is a flow
206+
/// removing the [key] from the map, bearing in mind that this is a flow
237207
/// map.
238-
SourceEdit _removeFromFlowMap(
239-
YamlEditor yamlEdit, YamlMap map, YamlNode keyNode, YamlNode valueNode) {
208+
SourceEdit _removeFromFlowMap(YamlEditor yamlEdit, YamlMap map, Object? key) {
209+
final (index: _, :keyNode, :valueNode) = getYamlMapEntry(map, key);
210+
240211
var start = keyNode.span.start.offset;
241212
var end = valueNode.span.end.offset;
242213
final yaml = yamlEdit.toString();

0 commit comments

Comments
 (0)