Refactor comment skipping

lib/src/editor.dart

@@ -241,11 +241,25 @@ 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: lineEnding,
+        greedy: true,
+      ).$1;
+      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);
     }
 
@@ -483,7 +497,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);

lib/src/equality.dart

@@ -87,8 +87,10 @@ int deepHashCode(Object? value) {
 }
 
 /// Returns the [YamlNode] corresponding to the provided [key].
-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

lib/src/list_mutations.dart

@@ -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);
@@ -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: lineEnding,
+      nodeToReplaceEndOffset: end,
+      update: newValue,
+      updateAsString: valueString,
+    );
+
     return SourceEdit(offset, end - offset, valueString);
   } else {
     valueString = yamlEncodeFlow(newValue);
@@ -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.

lib/src/map_mutations.dart

@@ -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) {
@@ -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);
@@ -127,12 +128,17 @@ 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);
+
+  final (_, 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)) {
@@ -150,9 +156,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: lineEnding);
+  end = offsetOfLastComment;
+
+  valueAsString = normalizeEncodedBlock(
+    yaml,
+    lineEnding: lineEnding,
+    nodeToReplaceEndOffset: end,
+    update: newValue,
+    updateAsString: valueAsString,
+  );
 
   return SourceEdit(start, end - start, valueAsString);
 }

lib/src/strings.dart

@@ -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);
@@ -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
@@ -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].
@@ -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.