-
Notifications
You must be signed in to change notification settings - Fork 273
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: extra space after position #732
fix: extra space after position #732
Conversation
@@ -115,7 +119,13 @@ export class SingleYAMLDocument extends JSONDocument { | |||
return; | |||
} | |||
|
|||
if (range[0] <= positionOffset && range[1] >= positionOffset) { | |||
const isNullNodeOnTheLine = (): boolean => |
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.
inside the function because the optimization
|
||
this.arrayPrefixIndentation = ''; | ||
let overwriteRange: Range = null; | ||
if (node && isScalar(node) && node.value === 'null') { | ||
if (areOnlySpacesAfterPosition) { | ||
overwriteRange = Range.create(position, Position.create(position.line, lineContent.length)); |
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.
'remove' extra space after cursor
if (node) { | ||
// when the value is null but the cursor is between prop name and null value (cursor is not at the end of the line) | ||
if (isMap(node) && node.items.length && isPair(node.items[0])) { |
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.
the mistake was to take only 0. index.
null node could be anywhere inside the collection. So there would have to be some complicated implementation to find the closest null node.
Same problem inside array fix
@@ -1306,7 +1306,7 @@ describe('Auto Completion Tests', () => { | |||
assert.equal(result.items.length, 1); | |||
assert.deepEqual( | |||
result.items[0], | |||
createExpectedCompletion('- (array item)', '- name: ${1:test}', 3, 4, 3, 4, 9, 2, { | |||
createExpectedCompletion('- (array item)', '- name: ${1:test}', 3, 4, 3, 5, 9, 2, { |
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.
4->5 because extra space will be removed after inserting the suggested item
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.
LGTM
What does this PR do?
fix another situation with extra space after the current cursor.
My previous fixes covered only some of the possible situations (#696, #697)
there were still problems in this type of yaml (2nd position)
This fix will solve this situation (for object, arrays in one place) by finding the correct node inside
getNodeFromPosition
, instead of fixing it later and trying to find a better node (null node).Another part of this PR is to overwrite range - remove extra space
What issues does this PR fix or reference?
no ref
Is it tested? How?
modified and added unit tests