-
Notifications
You must be signed in to change notification settings - Fork 16
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 splice list insertion #84
Changes from 11 commits
76df4d8
94c1d3b
905f77e
fe44d09
5f369b4
5978c20
f223d48
825b7cf
436a2b5
1b0c434
5e71dfb
9c735e5
66b25a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,7 +112,7 @@ SourceEdit _appendToFlowList( | |
/// block list. | ||
SourceEdit _appendToBlockList( | ||
YamlEditor yamlEdit, YamlList list, YamlNode item) { | ||
var formattedValue = _formatNewBlock(yamlEdit, list, item); | ||
var (formattedValue, _) = _formatNewBlock(yamlEdit, list, item); | ||
final yaml = yamlEdit.toString(); | ||
var offset = list.span.end.offset; | ||
|
||
|
@@ -132,7 +132,8 @@ SourceEdit _appendToBlockList( | |
} | ||
|
||
/// Formats [item] into a new node for block lists. | ||
String _formatNewBlock(YamlEditor yamlEdit, YamlList list, YamlNode item) { | ||
(String formatted, String indent) _formatNewBlock( | ||
YamlEditor yamlEdit, YamlList list, YamlNode item) { | ||
final yaml = yamlEdit.toString(); | ||
final listIndentation = getListIndentation(yaml, list); | ||
final newIndentation = listIndentation + getIndentation(yamlEdit); | ||
|
@@ -142,9 +143,12 @@ String _formatNewBlock(YamlEditor yamlEdit, YamlList list, YamlNode item) { | |
if (isCollection(item) && !isFlowYamlCollectionNode(item) && !isEmpty(item)) { | ||
valueString = valueString.substring(newIndentation); | ||
} | ||
final indentedHyphen = '${' ' * listIndentation}- '; | ||
|
||
return '$indentedHyphen$valueString$lineEnding'; | ||
// Pass back the indentation for use | ||
final hyphenIndentation = ' ' * listIndentation; | ||
final indentedHyphen = '$hyphenIndentation- '; | ||
|
||
return ('$indentedHyphen$valueString$lineEnding', hyphenIndentation); | ||
} | ||
|
||
/// Formats [item] into a new node for flow lists. | ||
|
@@ -172,14 +176,107 @@ SourceEdit _insertInBlockList( | |
|
||
if (index == list.length) return _appendToBlockList(yamlEdit, list, item); | ||
|
||
final formattedValue = _formatNewBlock(yamlEdit, list, item); | ||
var (formattedValue, indent) = _formatNewBlock(yamlEdit, list, item); | ||
|
||
final currNode = list.nodes[index]; | ||
final currNodeStart = currNode.span.start.offset; | ||
final yaml = yamlEdit.toString(); | ||
final start = yaml.lastIndexOf('\n', currNodeStart) + 1; | ||
|
||
return SourceEdit(start, 0, formattedValue); | ||
final currSequenceOffset = yaml.lastIndexOf('-', currNodeStart - 1); | ||
|
||
final (isNested, offset) = _isNestedInBlockList(currSequenceOffset, yaml); | ||
|
||
/// We have to get rid of the left indentation applied by default | ||
if (isNested && index == 0) { | ||
/// The [insertionIndex] will be equal to the start of | ||
/// [currentSequenceOffset] of the element we are inserting before in most | ||
/// cases. | ||
/// | ||
/// Example: | ||
/// | ||
/// - - value | ||
/// ^ Inserting before this and we get rid of indent | ||
/// | ||
/// If not, we need to account for the space between them that is not an | ||
/// indent. | ||
/// | ||
/// Example: | ||
/// | ||
/// - - value | ||
/// ^ Inserting before this and we get rid of indent. But also account | ||
/// for space in between | ||
final leftPad = currSequenceOffset - offset; | ||
final padding = ' ' * leftPad; | ||
|
||
/// Since we CANT'T/SHOULDN'T manipulate the next element to get rid of the | ||
/// space it has, we remove the padding (if any is present) from the indent | ||
/// itself. | ||
indent = indent.replaceFirst(padding, ''); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't we use But also if we made Or am I wrong here? I'm just wondering, if maybe it's smarter to return integers instead of strings, when we want to communicate indentation. We return an integer from Honestly, we could probably also just call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This actually makes sense. The |
||
|
||
// Give the indent to the first element | ||
formattedValue = '$padding${formattedValue.trimLeft()}$indent'; | ||
} | ||
|
||
return SourceEdit(offset, 0, formattedValue); | ||
} | ||
|
||
/// Determines if the list containing an element is nested within another list. | ||
/// The [currentSequenceOffset] indicates the index of the element's `-` and | ||
/// [yaml] represents the entire yaml document. | ||
/// | ||
/// ```yaml | ||
/// # Returns true | ||
/// - - value | ||
/// | ||
/// # Returns true | ||
/// - - value | ||
/// | ||
/// # Returns false | ||
/// key: | ||
/// - value | ||
/// | ||
/// # Returns false. Even though nested, a "\n" precedes the previous "-" | ||
/// - | ||
/// - value | ||
/// ``` | ||
(bool isNested, int offset) _isNestedInBlockList( | ||
int currentSequenceOffset, String yaml) { | ||
final startIndex = currentSequenceOffset - 1; | ||
|
||
/// Indicates the element we are inserting before is at index `0` of the list | ||
/// at the root of the yaml | ||
/// | ||
/// Example: | ||
/// | ||
/// - foo | ||
/// ^ Inserting before this | ||
if (startIndex < 0) return (false, 0); | ||
|
||
final newLineStart = yaml.lastIndexOf('\n', startIndex); | ||
final seqStart = yaml.lastIndexOf('-', startIndex); | ||
|
||
/// Indicates that a `\n` is closer to the last `-`. Meaning this list is not | ||
/// nested. | ||
/// | ||
/// Example: | ||
/// | ||
/// key: | ||
/// - value | ||
/// ^ Inserting before this and we need to keep the indent. | ||
/// | ||
/// Also this list may be nested but the nested list starts its indent after | ||
/// a new line. | ||
/// | ||
/// Example: | ||
/// | ||
/// - | ||
/// - value | ||
/// ^ Inserting before this and we need to keep the indent. | ||
if (newLineStart >= seqStart) { | ||
return (false, newLineStart + 1); | ||
} | ||
|
||
return (true, seqStart + 2); // Inclusive of space | ||
} | ||
|
||
/// Returns a [SourceEdit] describing the change to be made on [yamlEdit] to | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
SPLICE LIST IN A NESTED BLOCK LIST WITH WEIRD SPACES | ||
--- | ||
key: | ||
- - bar1 | ||
- bar2 | ||
- - foo | ||
- - baz | ||
--- | ||
- [splice, [key, 0], 0, 0, ['pre-bar1']] | ||
- [splice, [key, 0], 2, 0, ['post-bar2']] | ||
- [splice, [key, 2], 1, 0, ['post-baz']] | ||
- [splice, [key, 2], 0, 0, ['pre-baz']] | ||
- [splice, [key, 1], 0, 0, ['pre-foo']] | ||
|
||
jonasfj marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
SPLICE LIST IN A NESTED BLOCK LIST WITHOUT INITIAL SPACES | ||
--- | ||
key: | ||
- - bar1 | ||
- bar2 | ||
- - foo | ||
- - baz | ||
--- | ||
- [splice, [key, 0], 0, 0, ['pre-bar1']] | ||
- [splice, [key, 0], 2, 0, ['post-bar2']] | ||
- [splice, [key, 2], 1, 0, ['post-baz']] | ||
- [splice, [key, 2], 0, 0, ['pre-baz']] | ||
- [splice, [key, 1], 0, 0, ['pre-foo']] |
jonasfj marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
SLICE LIST IN NESTED BLOCK LIST | ||
--- | ||
key: | ||
- foo: | ||
- - bar: | ||
- - - false | ||
- - - false | ||
- - - false | ||
--- | ||
- [splice, [key], 0, 0, ['pre-foo']] | ||
- [splice, [key, 1, 'foo', 0], 0, 1, ['test']] | ||
- [splice, [key, 2], 0, 0, ['test']] | ||
- [splice, [key], 4, 1, ['tail-foo']] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
key: | ||
- - bar1 | ||
- bar2 | ||
- - foo | ||
- - baz | ||
--- | ||
key: | ||
- - pre-bar1 | ||
- bar1 | ||
- bar2 | ||
- - foo | ||
- - baz | ||
--- | ||
key: | ||
- - pre-bar1 | ||
- bar1 | ||
- post-bar2 | ||
- bar2 | ||
- - foo | ||
- - baz | ||
--- | ||
key: | ||
- - pre-bar1 | ||
- bar1 | ||
- post-bar2 | ||
- bar2 | ||
- - foo | ||
- - baz | ||
- post-baz | ||
--- | ||
key: | ||
- - pre-bar1 | ||
- bar1 | ||
- post-bar2 | ||
- bar2 | ||
- - foo | ||
- - pre-baz | ||
- baz | ||
- post-baz | ||
--- | ||
key: | ||
- - pre-bar1 | ||
- bar1 | ||
- post-bar2 | ||
- bar2 | ||
- - pre-foo | ||
- foo | ||
- - pre-baz | ||
- baz | ||
- post-baz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure it wouldn't make more sense to return
listIndentation
? Instead of returning the actual string.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll investigate that. I did that for convenience. I didn't want to change so many things in the existing code.