Conversation
Package publishingIf you have publishing permissions, you can use the links below to publish the changes after merging this PR.
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
PR HealthLicense Headers ✔️
All source files should start with a license header. Unrelated files missing license headers
This check can be disabled by tagging the PR with Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with Breaking changes ✔️
This check can be disabled by tagging the PR with API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
This check can be disabled by tagging the PR with Unused Dependencies ✔️
For details on how to fix these, see dependency_validator. This check can be disabled by tagging the PR with
Coverage
|
| File | Coverage |
|---|---|
| pkgs/yaml_edit/lib/src/char_codes.dart | 💚 100 % |
| pkgs/yaml_edit/lib/src/list_mutations.dart | 💚 99 % ⬆️ 0 % |
| pkgs/yaml_edit/lib/src/map_mutations.dart | 💔 99 % ⬇️ 1 % |
| pkgs/yaml_edit/lib/src/utils.dart | 💔 91 % ⬇️ 3 % |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes flow list and flow map mutations when comments contain delimiter characters, and adds support for updating empty values in flow maps without whitespace after the colon. It introduces a new YamlChar utility class and robust delimiter-finding helper functions (findNextFlowDelimiter and findPreviousFlowDelimiter) that correctly skip comments and quoted strings. The reviewer suggested replacing remaining magic numbers (like 0x27, 0x22, and 0x5C) in the delimiter-finding logic with new constants in YamlChar to improve consistency and readability.
| int _findCommentStartOnLine(String yaml, int lineStart, int lineEnd) { | ||
| var inSingleQuote = false; | ||
| var inDoubleQuote = false; | ||
| for (var i = lineStart; i < lineEnd; i++) { | ||
| final c = yaml.codeUnitAt(i); | ||
| final prev = i > lineStart ? yaml.codeUnitAt(i - 1) : YamlChar.space; | ||
|
|
||
| if (inSingleQuote) { | ||
| if (c == 0x27 /* ' */) { | ||
| if (i + 1 < lineEnd && yaml.codeUnitAt(i + 1) == 0x27) { | ||
| i++; | ||
| } else { | ||
| inSingleQuote = false; | ||
| } | ||
| } | ||
| } else if (inDoubleQuote) { | ||
| if (c == 0x5C /* \ */) { | ||
| i++; | ||
| } else if (c == 0x22 /* " */) { | ||
| inDoubleQuote = false; | ||
| } | ||
| } else { | ||
| final isQuoteStart = i == lineStart || | ||
| YamlChar.isWhitespace(prev) || | ||
| YamlChar.isFlowIndicator(prev) || | ||
| prev == YamlChar.colon; | ||
|
|
||
| if (c == 0x27 /* ' */ && isQuoteStart) { | ||
| inSingleQuote = true; | ||
| } else if (c == 0x22 /* " */ && isQuoteStart) { | ||
| inDoubleQuote = true; | ||
| } else if (c == YamlChar.hash) { | ||
| if (i == lineStart || | ||
| YamlChar.isWhitespace(prev) || | ||
| YamlChar.isLineBreak(prev)) { | ||
| return i; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
You've introduced a very helpful YamlChar class to manage character codes, which is great for readability and maintainability. However, there are still some magic numbers used for quote and backslash characters in this function and in findNextFlowDelimiter.
To improve consistency, consider adding constants for single quote (0x27), double quote (0x22), and backslash (0x5C) to the YamlChar class. Then, you can replace the magic numbers in this file with these new constants.
For example, you could add the following to pkgs/yaml_edit/lib/src/char_codes.dart:
// ...
/// Character code for double quote (`"`).
static const int doubleQuote = 0x22;
/// Character code for single quote (`'`).
static const int singleQuote = 0x27;
/// Character code for backslash (`\`).
static const int backslash = 0x5C;
// ...And then use them here and in findNextFlowDelimiter:
// ...
if (c == YamlChar.singleQuote) {
// ...There was a problem hiding this comment.
Resolved in commit fc17034. Added doubleQuote, singleQuote, and backslash to YamlChar and replaced the magic numbers in utils.dart.
1 participant
This PR adds comment-aware flow delimiter scanning and fixes updating empty values in flow maps in
package:yaml_edit.Changes
YamlCharconstants and helper predicates inlib/src/char_codes.dart.findNextFlowDelimiterandfindPreviousFlowDelimiterinlib/src/utils.dartthat skip# ...comments when searching for flow delimiters (,,],})._insertInFlowList), flow list removals (_removeFromFlowList), and flow map removals (_removeFromFlowMap)._replaceInFlowMap, handle empty flow map values ({a:}) without leaving syntax errors when replacing with a value.test/insert_test.dart,test/remove_test.dart,test/update_test.dart, andtest/utils_test.dart.