Fix line folding 2 #88
Conversation
> Fix invalid encoding for string with trailing line breaks or white-space
> Add a string test case in `string_tests.dart`
> Update `_tryYamlEncodedPlain` to return null if string cannot be encoded > Make `_yamlEncodeFlowScalar` and `yamlEncodeBlockScalar` code concise
> Check for trailing whitespace rather than `\n `.
> `_yamlEncodeFlowScalar` and `_yamlEncodeBlockScalar` always encode YamlScalars
| if (value.style == ScalarStyle.SINGLE_QUOTED) { | ||
| return _tryYamlEncodeSingleQuoted(val); | ||
| } | ||
| // TODO: Are there other strings we can't encode in folded mode? |
There was a problem hiding this comment.
There must be other things we can't represent in folded mode, right?
There was a problem hiding this comment.
The leading space was the main culprit.
Also the if statement for the leading space and line break (in view here) makes the tests fail 😅 I will carry on from your commit.
| /// ``` | ||
| /// | ||
| /// See: https://yaml.org/spec/1.2.2/#813-folded-style | ||
| String? _tryYamlEncodeFolded(String string, int indentSize, String lineEnding) { |
There was a problem hiding this comment.
Also I'm realizing that we might be missing the point with folded mode.
The point with folded mode is that we can replace a b with a\nb and still get the same value.
If the string contains newlines then this becomes complicated, because a newline essentially have to be replaced by an empty-line, right?
But also, rather importantly: the point of a folded string is to be able to wrap long lines without affecting the end-result.
A: |-
my very long string that cannot be broken, if I don't want newlines.
# As folded string
B: >-
my very long string that
cannot be broken, if I
don't want newlines.In this case (A) and (B) are the same. The reason for using folded strings is that you don't want VERY long lines.
So if we want to support folded string, I guess we ought to maybe wrap strings at 80 characters, or something like that?
This is perhaps something we can do in a follow up PR, if we just make the folded string logic work correctly, then perhaps improving the formatting is a follow up.
There was a problem hiding this comment.
So if we want to support folded string, I guess we ought to maybe wrap strings at 80 characters, or something like that?
That's a fairly tricky (but achievable) implementation that's why I initially asked this in the comment. It seems I didn't frame it correctly. I meant it like,
"Should we leave it as-is with the formatting we do or mutate it to match yaml while still holding the same meaning" when parsed.
If the string contains newlines then this becomes complicated, because a newline essentially have to be replaced by an empty-line, right?
\n have to be \n\n in folded to hold the same meaning unless the next line starts with leading space then we can let it slide.
☝🏽 That makes it a bit hard to apply to the first line that starts with a leading space. Since at this point package: yaml won't have the indent information. That's why explicitly fall back to doubleQuoted. Will work on it next.
| /// Simplest block style. | ||
| /// * https://yaml.org/spec/1.2.2/#812-literal-style | ||
| return '|${_getChompingIndicator(string)}\n$indent' | ||
| return '|${string.endsWith('\n') ? '' : '-'}\n$indent' | ||
| '${string.replaceAll('\n', lineEnding + indent)}'; | ||
| } | ||
|
|
There was a problem hiding this comment.
@jonasfj I finally got around to look into this. Why the change? Literal strings are also subject to chomping as indicated here
This causes anything with more than one trailing \n to fail as every new line that's empty will be clipped such that:
foo bar \nremains asfoo bar \n(okay)foo bar \n\nbecomesfoo bar \n(not okay).
That's why I went for a wildcard approach. We chomp so that it's always parsed correctly by package: yaml even if we only have a single line break.
|
Closing in favor of #87 |
@kekavc24 Í tried to play a bit with the things you did, and refactored a few things.
I'm pretty sure I also broke a lot of tests 🙈
Feel free to ignore or take inspiration.