Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix line ending bug #90

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7501cb4
Add function to skip and/or extract comments
kekavc24 Jun 30, 2024
647329c
Add function to normalize trailing line breaks in encode block
kekavc24 Jul 1, 2024
502fa0a
Return index getting key node in map
kekavc24 Jul 1, 2024
c3056c8
Apply line-break after each encoded yaml block
kekavc24 Jul 1, 2024
e610993
Encode folded/literal strings based on c3056c8
kekavc24 Jul 1, 2024
e659cb9
Skip comments and include `\n` in map mutations
kekavc24 Jul 1, 2024
53f9637
Skip comments and remove additional `\n` added in list mutations
kekavc24 Jul 1, 2024
edd8d38
Normalize top level edits
kekavc24 Jul 1, 2024
f5a259b
Remove defensive encoding function after fix in e659cb9 and 53f9637
kekavc24 Jul 1, 2024
5a51cbe
Run dart format
kekavc24 Jul 1, 2024
c7aec85
Skip comments for top-level edits
kekavc24 Jul 2, 2024
157063a
Lazily look ahead for comments
kekavc24 Jul 2, 2024
f7fe2d3
Refactor function to normalize encoded blocks
kekavc24 Jul 2, 2024
9101d79
Prevent pruning in YamlScalar with ScalarStyles plain, any, folded, l…
kekavc24 Jul 3, 2024
3d99caf
Allow comments to be skipped greedily or lazily
kekavc24 Jul 3, 2024
4034652
Ensure `_appendToBlockList` appends after last comment
kekavc24 Jul 3, 2024
7ada07e
Fix issue where loop never exits
kekavc24 Jul 3, 2024
ee6a29e
Avoid skipping line break eagerly when extracting comments
kekavc24 Jul 4, 2024
422731d
Add utility method to reclaim indent skipped
kekavc24 Jul 4, 2024
f3a265e
Refactor `_removeFromBlockList` to correctly skip comments
kekavc24 Jul 4, 2024
814672b
Refactor `_removeFromBlockMap` to correctly skip comments
kekavc24 Jul 4, 2024
7378e92
Use span length to determine true state of `null`
kekavc24 Jul 4, 2024
7332de6
Improve comments describing chomping hack
kekavc24 Oct 24, 2024
6619266
Move extraction of terminal YamlScalar to utility function
kekavc24 Oct 24, 2024
a87b3fe
Refactor `skipAndExtractComments` function
kekavc24 Oct 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions lib/src/editor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,26 @@ class YamlEditor {

if (path.isEmpty) {
final start = _contents.span.start.offset;
final end = getContentSensitiveEnd(_contents);
var end = getContentSensitiveEnd(_contents);
final lineEnding = getLineEnding(_yaml);
final edit = SourceEdit(
start, end - start, yamlEncodeBlock(valueNode, 0, lineEnding));

end = skipAndExtractCommentsInBlock(
_yaml,
endOfNodeOffset: end,
lineEnding: lineEnding,
greedy: true,
).endOffset;

var encoded = yamlEncodeBlock(valueNode, 0, lineEnding);
encoded = normalizeEncodedBlock(
_yaml,
lineEnding: lineEnding,
nodeToReplaceEndOffset: end,
update: valueNode,
updateAsString: encoded,
skipPreservationCheck: true,
);
final edit = SourceEdit(start, end - start, encoded);
return _performEdit(edit, path, valueNode);
}

Expand Down Expand Up @@ -483,7 +498,7 @@ class YamlEditor {
if (!containsKey(map, keyOrIndex)) {
return _pathErrorOrElse(path, path.take(i + 1), map, orElse);
}
final keyNode = getKeyNode(map, keyOrIndex);
final (_, keyNode) = getKeyNode(map, keyOrIndex);

if (checkAlias) {
if (_aliases.contains(keyNode)) throw AliasException(path, keyNode);
Expand Down
6 changes: 4 additions & 2 deletions lib/src/equality.dart
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,10 @@ int deepHashCode(Object? value) {
}

/// Returns the [YamlNode] corresponding to the provided [key].
kekavc24 marked this conversation as resolved.
Show resolved Hide resolved
YamlNode getKeyNode(YamlMap map, Object? key) {
return map.nodes.keys.firstWhere((node) => deepEquals(node, key)) as YamlNode;
(int index, YamlNode keyNode) getKeyNode(YamlMap map, Object? key) {
return map.nodes.keys.indexed.firstWhere(
(value) => deepEquals(value.$2, key),
) as (int, YamlNode);
}

/// Returns the [YamlNode] after the [YamlNode] corresponding to the provided
Expand Down
207 changes: 129 additions & 78 deletions lib/src/list_mutations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,21 @@ SourceEdit updateInList(
final listIndentation = getListIndentation(yaml, list);
final indentation = listIndentation + getIndentation(yamlEdit);
final lineEnding = getLineEnding(yaml);
valueString =
yamlEncodeBlock(wrapAsYamlNode(newValue), indentation, lineEnding);

final encoded = yamlEncodeBlock(
wrapAsYamlNode(newValue),
indentation,
lineEnding,
);
valueString = encoded;

/// We prefer the compact nested notation for collections.
///
/// By virtue of [yamlEncodeBlockString], collections automatically
/// By virtue of [yamlEncodeBlock], collections automatically
/// have the necessary line endings.
if ((newValue is List && (newValue as List).isNotEmpty) ||
(newValue is Map && (newValue as Map).isNotEmpty)) {
valueString = valueString.substring(indentation);
} else if (currValue.collectionStyle == CollectionStyle.BLOCK) {
valueString += lineEnding;
}

var end = getContentSensitiveEnd(currValue);
Expand All @@ -50,6 +53,21 @@ SourceEdit updateInList(
valueString = ' $valueString';
}

// Aggressively skip all comments
end = skipAndExtractCommentsInBlock(
yaml,
endOfNodeOffset: end,
lineEnding: lineEnding,
).endOffset;

valueString = normalizeEncodedBlock(
yaml,
lineEnding: lineEnding,
nodeToReplaceEndOffset: end,
update: newValue,
updateAsString: valueString,
);

return SourceEdit(offset, end - offset, valueString);
} else {
valueString = yamlEncodeFlow(newValue);
Expand Down Expand Up @@ -112,22 +130,41 @@ SourceEdit _appendToFlowList(
/// block list.
SourceEdit _appendToBlockList(
YamlEditor yamlEdit, YamlList list, YamlNode item) {
var (indentSize, valueToIndent) = _formatNewBlock(yamlEdit, list, item);
var formattedValue = '${' ' * indentSize}$valueToIndent';
/// A block list can never be empty since a `-` must be seen for it to be a
/// valid block sequence.
///
/// See description of:
/// https://yaml.org/spec/1.2.2/#82-block-collection-styles.
assert(
list.isNotEmpty,
'A YamlList encoded as CollectionStyle.BLOCK must have a value',
);

final yaml = yamlEdit.toString();
var offset = list.span.end.offset;
final lineEnding = getLineEnding(yaml);

// Adjusts offset to after the trailing newline of the last entry, if it
// exists
if (list.isNotEmpty) {
final lastValueSpanEnd = list.nodes.last.span.end.offset;
final nextNewLineIndex = yaml.indexOf('\n', lastValueSpanEnd - 1);
if (nextNewLineIndex == -1) {
formattedValue = getLineEnding(yaml) + formattedValue;
} else {
offset = nextNewLineIndex + 1;
}
// Lazily skip all comments and white-space at the end.
final offset = skipAndExtractCommentsInBlock(
yaml,
endOfNodeOffset: list.nodes.last.span.end.offset,
lineEnding: lineEnding,
).endOffset;

var (indentSize, formattedValue) = _formatNewBlock(yamlEdit, list, item);

formattedValue = normalizeEncodedBlock(
yaml,
lineEnding: lineEnding,
nodeToReplaceEndOffset: offset,
update: item,
updateAsString: formattedValue,
);

formattedValue = '${' ' * indentSize}$formattedValue';

// Apply line ending incase it's missing
if (yaml[offset - 1] != '\n') {
formattedValue = '$lineEnding$formattedValue';
}

return SourceEdit(offset, 0, formattedValue);
Expand All @@ -146,7 +183,7 @@ SourceEdit _appendToBlockList(
valueString = valueString.substring(newIndentation);
}

return (listIndentation, '- $valueString$lineEnding');
return (listIndentation, '- $valueString');
}

/// Formats [item] into a new node for flow lists.
Expand Down Expand Up @@ -239,7 +276,7 @@ SourceEdit _insertInBlockList(
/// ```
(bool isNested, int offset) _isNestedInBlockList(
int currentSequenceOffset, String yaml) {
final startIndex = currentSequenceOffset - 1;
final startOffset = currentSequenceOffset - 1;

/// Indicates the element we are inserting before is at index `0` of the list
/// at the root of the yaml
Expand All @@ -248,10 +285,10 @@ SourceEdit _insertInBlockList(
///
/// - foo
/// ^ Inserting before this
if (startIndex < 0) return (false, 0);
if (startOffset < 0) return (false, 0);

final newLineStart = yaml.lastIndexOf('\n', startIndex);
final seqStart = yaml.lastIndexOf('-', startIndex);
final newLineStart = yaml.lastIndexOf('\n', startOffset);
final seqStart = yaml.lastIndexOf('-', startOffset);

/// Indicates that a `\n` is closer to the last `-`. Meaning this list is not
/// nested.
Expand Down Expand Up @@ -308,70 +345,84 @@ SourceEdit _removeFromBlockList(
YamlEditor yamlEdit, YamlList list, YamlNode nodeToRemove, int index) {
RangeError.checkValueInInterval(index, 0, list.length - 1);

var end = getContentSensitiveEnd(nodeToRemove);

/// If we are removing the last element in a block list, convert it into a
/// flow empty list.
if (list.length == 1) {
final start = list.span.start.offset;
final yaml = yamlEdit.toString();
final yamlSize = yaml.length;

return SourceEdit(start, end - start, '[]');
final lineEnding = getLineEnding(yaml);
final YamlNode(:span) = nodeToRemove;

var startOffset = span.start.offset;
startOffset =
span.length == 0 ? startOffset : yaml.lastIndexOf('-', startOffset - 1);

var endOffset = getContentSensitiveEnd(nodeToRemove);

/// YamlMap may have `null` value for the last key and we need to ensure the
/// correct [endOffset] is provided to [skipAndExtractCommentsInBlock],
/// otherwise [skipAndExtractCommentsInBlock] may prematurely return an
/// incorrect offset because it immediately saw `:`
if (nodeToRemove is YamlMap &&
endOffset < yamlSize &&
nodeToRemove.nodes.entries.last.value.value == null) {
endOffset += 1;
}

final yaml = yamlEdit.toString();
final span = nodeToRemove.span;
// We remove any content belonging to [nodeToRemove] greedily
endOffset = skipAndExtractCommentsInBlock(
yaml,
endOfNodeOffset: endOffset == startOffset ? endOffset + 1 : endOffset,
lineEnding: lineEnding,
greedy: true,
).endOffset;

/// Adjust the end to clear the new line after the end too.
///
/// We do this because we suspect that our users will want the inline
/// comments to disappear too.
final nextNewLine = yaml.indexOf('\n', end);
if (nextNewLine != -1) {
end = nextNewLine + 1;
}
final listSize = list.length;

final isSingleElement = listSize == 1;
final isLastElementInList = index == listSize - 1;
final isLastInYaml = endOffset == yamlSize;

final replacement = isSingleElement ? '[]' : '';

/// If the value is empty
if (span.length == 0) {
var start = span.start.offset;
return SourceEdit(start, end - start, '');
/// Adjust [startIndent] to include any indent this element may have had
/// to prevent it from interfering with the indent of the next [YamlNode]
/// which isn't in this list. We move it back if:
/// 1. The [nodeToRemove] is the last element in a [list] with more than
/// one element.
/// 2. It also isn't the first element in the yaml.
///
/// Doing this only for the last element ensures that any value's indent is
/// automatically given to the next element in the list such that,
///
/// 1. If nested:
/// - - value
/// ^ This space goes to the next element that ends up here
///
/// 2. If not nested, then the next element gets the indent if any is present.
if (isLastElementInList && startOffset != 0 && !isSingleElement) {
final index = yaml.lastIndexOf('\n', startOffset);
startOffset = index == -1 ? startOffset : index + 1;
}

/// -1 accounts for the fact that the content can start with a dash
var start = yaml.lastIndexOf('-', span.start.offset - 1);

/// Check if there is a `-` before the node
if (start > 0) {
final lastHyphen = yaml.lastIndexOf('-', start - 1);
final lastNewLine = yaml.lastIndexOf('\n', start - 1);
if (lastHyphen > lastNewLine) {
start = lastHyphen + 2;

/// If there is a `-` before the node, we need to check if we have
/// to update the indentation of the next node.
if (index < list.length - 1) {
/// Since [end] is currently set to the next new line after the current
/// node, check if we see a possible comment first, or a hyphen first.
/// Note that no actual content can appear here.
///
/// We check this way because the start of a span in a block list is
/// the start of its value, and checking from the back leaves us
/// easily confused if there are comments that have dashes in them.
final nextHash = yaml.indexOf('#', end);
final nextHyphen = yaml.indexOf('-', end);
final nextNewLine = yaml.indexOf('\n', end);

/// If [end] is on the same line as the hyphen of the next node
if ((nextHash == -1 || nextHyphen < nextHash) &&
nextHyphen < nextNewLine) {
end = nextHyphen;
}
}
} else if (lastNewLine > lastHyphen) {
start = lastNewLine + 1;
}
/// We intentionally [skipAndExtractCommentsInBlock] greedily which also
/// consumes the next [YamlNode]'s indent.
///
/// For elements at the last index, we need to reclaim the indent belonging
/// to the next node not in the list and optionally include a line break if
/// if it is the only element. See [reclaimIndentAndLinebreak] for more info.
if (isLastElementInList && !isLastInYaml) {
endOffset = reclaimIndentAndLinebreak(
yaml,
endOffset,
isSingle: isSingleElement,
);
} else if (isLastInYaml && yaml[endOffset - 1] == '\n' && isSingleElement) {
/// Remove any dangling line-break that may have been part of the yaml:
/// -`\r\n` = 2
/// - `\n` = 1
endOffset -= lineEnding == '\n' ? 1 : 2;
}

return SourceEdit(start, end - start, '');
return SourceEdit(startOffset, endOffset - startOffset, replacement);
}

/// Returns a [SourceEdit] describing the change to be made on [yamlEdit] to
Expand Down
Loading
Loading