Skip to content

Conversation

ogorodnik
Copy link
Contributor

No description provided.

Copy link
Collaborator

@AnthonyLeonardoGracio AnthonyLeonardoGracio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let Vadim comment on the VSS part in itself, since I am not an expert. We miss some tests too

Out_Span,
Edit);

if Span.last.line - Span.first.line > 5 then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a comment explaining this threshold

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a space at the end of comment, I guess

Span : LSP.Messages.Span;
New_Text : VSS.Strings.Virtual_String;
Edit : out LSP.Messages.TextEdit_Vector);
-- Create a diff between document Text inside Span and New_Chunk and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't explain the difference with the 'Diff' function here... You should explain why the TextEdits will be different (i.e: it will not replace the whole line a by a new line, but only add/delete some characters when possible).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

godunko
godunko previously approved these changes Jul 11, 2022
Copy link
Contributor

@godunko godunko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Copy link
Collaborator

@AnthonyLeonardoGracio AnthonyLeonardoGracio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's have a test on a rangeFormatting requet then

@ogorodnik ogorodnik force-pushed the V628-024 branch 2 times, most recently from ad3a3fd to 2810233 Compare July 15, 2022 16:01
Copy link
Collaborator

@AnthonyLeonardoGracio AnthonyLeonardoGracio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is really hard to read IMO. Maybe it's juste me, but I think you can try to improve a bit more the comments and the function names if other people need to resolve a bug later in that area.

@AnthonyLeonardoGracio
Copy link
Collaborator

The CI does not pass, please check again

@ogorodnik ogorodnik closed this Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants