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

Closed
wants to merge 11 commits into from
12 changes: 7 additions & 5 deletions lib/src/editor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,13 @@ 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, end, null, lineEnding).$1;
Copy link
Member

@jonasfj jonasfj Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
end = skipAndExtractCommentsInBlock(_yaml, end, null, lineEnding).$1;
final (endOffset, _) = skipAndExtractCommentsInBlock(_yaml, end, null, lineEnding);
end = endOffset;

or similar, but let's avoid .$1 if we can (inside a simple helper method like getKeyNode using .$2 is probably not so bad, but elsewhere, especially in larger code blocks, I'd suggest avoiding it).

Or make skipAndExtractCommentsInBlock return a record with named properties. So we can do: skipAndExtractCommentsInBlock(...).endOffset;.

var encoded = yamlEncodeBlock(valueNode, 0, lineEnding);
encoded =
normalizeEncodedBlock(_yaml, lineEnding, end, valueNode, encoded);
final edit = SourceEdit(start, end - start, encoded);
return _performEdit(edit, path, valueNode);
}

Expand Down Expand Up @@ -483,7 +485,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].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider updating the documentation.

Also feel free to submit this as a separate PR, and hint that you plan on using it in this PR. Then we can reduce the size of this PR a bit.

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
28 changes: 22 additions & 6 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,19 @@ SourceEdit updateInList(
valueString = ' $valueString';
}

// Aggressively skip all comments
final (offsetOfLastComment, _) =
skipAndExtractCommentsInBlock(yaml, end, null, lineEnding);
end = offsetOfLastComment;

valueString =
normalizeEncodedBlock(yaml, lineEnding, end, newValue, valueString);

/// [skipAndExtractCommentsInBlock] is greedy and eats up any whitespace
/// it encounters in search of comments. Compensate indent lost in the
/// current edit
if (index != list.length - 1) valueString += ' ' * listIndentation;

return SourceEdit(offset, end - offset, valueString);
} else {
valueString = yamlEncodeFlow(newValue);
Expand Down Expand Up @@ -146,7 +162,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
65 changes: 53 additions & 12 deletions lib/src/map_mutations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ SourceEdit updateInMap(
/// removing the element at [key] when re-parsed.
SourceEdit removeInMap(YamlEditor yamlEdit, YamlMap map, Object? key) {
assert(containsKey(map, key));
final keyNode = getKeyNode(map, key);
final (_, keyNode) = getKeyNode(map, key);
final valueNode = map.nodes[keyNode]!;

if (map.style == CollectionStyle.FLOW) {
Expand Down Expand Up @@ -83,13 +83,14 @@ SourceEdit _addToBlockMap(
}
}

var valueString = yamlEncodeBlock(newValue, newIndentation, lineEnding);
final valueString = yamlEncodeBlock(newValue, newIndentation, lineEnding);

if (isCollection(newValue) &&
!isFlowYamlCollectionNode(newValue) &&
!isEmpty(newValue)) {
formattedValue += '$keyString:$lineEnding$valueString$lineEnding';
formattedValue += '$keyString:$lineEnding$valueString';
} else {
formattedValue += '$keyString: $valueString$lineEnding';
formattedValue += '$keyString: $valueString';
}

return SourceEdit(offset, 0, formattedValue);
Expand Down Expand Up @@ -127,12 +128,18 @@ SourceEdit _replaceInBlockMap(
YamlEditor yamlEdit, YamlMap map, Object? key, YamlNode newValue) {
final yaml = yamlEdit.toString();
final lineEnding = getLineEnding(yaml);
final newIndentation =
getMapIndentation(yaml, map) + getIndentation(yamlEdit);
final mapIndentation = getMapIndentation(yaml, map);
final newIndentation = mapIndentation + getIndentation(yamlEdit);

// TODO: Compensate for the indent eaten up
final (keyIndex, keyNode) = getKeyNode(map, key);

var valueAsString = yamlEncodeBlock(
wrapAsYamlNode(newValue),
newIndentation,
lineEnding,
);

final keyNode = getKeyNode(map, key);
var valueAsString =
yamlEncodeBlock(wrapAsYamlNode(newValue), newIndentation, lineEnding);
if (isCollection(newValue) &&
!isFlowYamlCollectionNode(newValue) &&
!isEmpty(newValue)) {
Expand All @@ -150,9 +157,43 @@ SourceEdit _replaceInBlockMap(
var end = getContentSensitiveEnd(map.nodes[key]!);

/// `package:yaml` parses empty nodes in a way where the start/end of the
/// empty value node is the end of the key node, so we have to adjust for
/// this.
if (end < start) end = start;
/// empty value node is the end of the key node.
///
/// In our case, we need to ensure that any line-breaks are included in the
/// edit such that:
/// 1. We account for `\n` after a key within other keys or at the start
/// Example..
/// a:
/// b: value
///
/// or..
/// a: value
/// b:
/// c: value
///
/// 2. We don't suggest edits that are not within the string bounds because
/// of the `\n` we need to account for in Rule 1 above. This could be a
/// key:
/// * At the index `0` but it's the only key
/// * At the end in a map with more than one key
end = start == yaml.length
? start
: end < start
? start + 1
: end;

// Aggressively skip all comments
final (offsetOfLastComment, _) =
skipAndExtractCommentsInBlock(yaml, end, null, lineEnding);
end = offsetOfLastComment;

valueAsString =
normalizeEncodedBlock(yaml, lineEnding, end, newValue, valueAsString);

/// [skipAndExtractCommentsInBlock] is greedy and eats up any whitespace
/// it encounters in search of comments. Compensate indent lost in the
/// current edit
if (keyIndex != map.length - 1) valueAsString += ' ' * mapIndentation;

return SourceEdit(start, end - start, valueAsString);
}
Expand Down
117 changes: 79 additions & 38 deletions lib/src/strings.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ String? _tryYamlEncodeFolded(String string, int indentSize, String lineEnding) {

/// Remove trailing `\n` & white-space to ease string folding
var trimmed = string.trimRight();
final stripped = string.substring(trimmed.length);
var stripped = string.substring(trimmed.length);

final trimmedSplit =
trimmed.replaceAll('\n', lineEnding + indent).split(lineEnding);
Expand Down Expand Up @@ -137,9 +137,30 @@ String? _tryYamlEncodeFolded(String string, int indentSize, String lineEnding) {
return previous + lineEnding + updated;
});

return '>-\n'
stripped = stripped.replaceAll('\n', lineEnding); // Mild paranoia
final ignoreTrailingLineBreak = stripped.endsWith(lineEnding);

// We ignore it with conviction as explained below.
if (ignoreTrailingLineBreak) {
stripped = stripped.substring(0, stripped.length - 1);
}

/// If indeed we have a trailing line, we apply a `chomping hack`. We use a
/// `clip indicator` (no chomping indicator) if we need to ignore the `\n`
/// and `strip indicator` if not to remove any trailing indents.
///
/// The caller of this method, that is, [yamlEncodeBlock] will apply a
/// dangling `\n` that will\should be normalized by
/// [normalizeEncodedBlock] which allows trailing `\n` for [folded]
/// strings such that:
/// * If we had a string `"my string \n"`:
/// 1. This function excludes it and it becomes `>\n<indent>my string `
/// 2. [yamlEncodeBlock] applies `\n` that we skipped.
/// 2. [normalizeEncodedBlock] ignores the trailing `\n` for folded
/// string by default.
return '>${ignoreTrailingLineBreak ? '' : '-'}\n'
'$indent$trimmed'
'${stripped.replaceAll('\n', lineEnding + indent)}';
'${stripped.replaceAll(lineEnding, lineEnding + indent)}';
}

/// Attempts to encode a [string] as a _YAML literal string_ and apply the
Expand Down Expand Up @@ -170,13 +191,41 @@ String? _tryYamlEncodeLiteral(
// encoded in literal mode.
if (_hasUnprintableCharacters(string)) return null;

final indent = ' ' * indentSize;

// TODO: Are there other strings we can't encode in literal mode?
final trimmed = string.trimRight();

final indent = ' ' * indentSize;
// Mild paranoia
var stripped = string
.substring(
trimmed.length,
)
.replaceAll('\n', lineEnding);

final ignoreTrailingLineBreak = stripped.endsWith(lineEnding);

// We ignore it with conviction as explained below.
if (ignoreTrailingLineBreak) {
stripped = stripped.substring(0, stripped.length - 1);
}

/// Simplest block style.
/// * https://yaml.org/spec/1.2.2/#812-literal-style
return '|-\n$indent${string.replaceAll('\n', lineEnding + indent)}';
/// If indeed we have a trailing line, we apply a `chomping hack`. We use a
/// `clip indicator` (no chomping indicator) if we need to ignore the `\n`
/// and `strip indicator` if not to remove any trailing indents.
///
/// The caller of this method, that is, [yamlEncodeBlock] will apply a
/// dangling `\n` that will\should be normalized by
/// [normalizeEncodedBlock] which allows trailing `\n` for [literal]
/// strings such that:
/// * If we had a string `"my string \n"`:
/// 1. This function excludes it and it becomes `|\n<indent>my string `
/// 2. [yamlEncodeBlock] applies `\n` that we skipped.
/// 2. [normalizeEncodedBlock] ignores the trailing `\n` for literal
/// string by default.
return '|${ignoreTrailingLineBreak ? '' : '-'}\n'
'$indent${trimmed.replaceAll('\n', lineEnding + indent)}'
'${stripped.replaceAll(lineEnding, lineEnding + indent)}';
}

/// Encodes a flow [YamlScalar] based on the provided [YamlScalar.style].
Expand Down Expand Up @@ -276,64 +325,56 @@ String yamlEncodeFlow(YamlNode value) {
}

/// Returns [value] with the necessary formatting applied in a block context.
String yamlEncodeBlock(
YamlNode value,
int indentation,
String lineEnding,
) {
///
/// It is recommended that callers of this method also make a call to
/// [normalizeEncodedBlock] with this [value] as the `update` and output
/// of this call as the `updateAsString` to prune any dangling line-break.
String yamlEncodeBlock(YamlNode value, int indentation, String lineEnding) {
const additionalIndentation = 2;

if (!isBlockNode(value)) return yamlEncodeFlow(value);
if (!isBlockNode(value)) return yamlEncodeFlow(value) + lineEnding;

final newIndentation = indentation + additionalIndentation;

if (value is YamlList) {
if (value.isEmpty) return '${' ' * indentation}[]';
if (value.isEmpty) return '${' ' * indentation}[]$lineEnding';

Iterable<String> safeValues;
return value.nodes.fold('', (string, element) {
var valueString = yamlEncodeBlock(element, newIndentation, lineEnding);

final children = value.nodes;

safeValues = children.map((child) {
var valueString = yamlEncodeBlock(child, newIndentation, lineEnding);
if (isCollection(child) && !isFlowYamlCollectionNode(child)) {
if (isCollection(element) && !isFlowYamlCollectionNode(element)) {
valueString = valueString.substring(newIndentation);
}

return '${' ' * indentation}- $valueString';
return '$string${' ' * indentation}- $valueString';
});

return safeValues.join(lineEnding);
} else if (value is YamlMap) {
if (value.isEmpty) return '${' ' * indentation}{}';
if (value.isEmpty) return '${' ' * indentation}{}$lineEnding';

return value.nodes.entries.map((entry) {
return value.nodes.entries.fold('', (string, entry) {
final MapEntry(:key, :value) = entry;

final safeKey = yamlEncodeFlow(key as YamlNode);
final formattedKey = ' ' * indentation + safeKey;
var formattedKey = ' ' * indentation + safeKey;

final formattedValue = yamlEncodeBlock(
value,
newIndentation,
lineEnding,
);
final formattedValue = yamlEncodeBlock(value, newIndentation, lineEnding);

/// Empty collections are always encoded in flow-style, so new-line must
/// be avoided
if (isCollection(value) && !isEmpty(value)) {
return '$formattedKey:$lineEnding$formattedValue';
}
/// be avoided. Otherwise, begin the collection on a new line.
formattedKey = '$formattedKey:'
'${isCollection(value) && !isEmpty(value) ? lineEnding : " "}';

return '$formattedKey: $formattedValue';
}).join(lineEnding);
return '$string$formattedKey$formattedValue';
});
}

return _yamlEncodeBlockScalar(
final encodedScalar = _yamlEncodeBlockScalar(
value as YamlScalar,
newIndentation,
lineEnding,
);

return encodedScalar + lineEnding;
}

/// List of unprintable characters.
Expand Down
Loading
Loading