Skip to content

Return \n to end of line in case it's deleted #197

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

Rutherther
Copy link
Contributor

One possible way to resolve #193.

It looks to me like LSP really does delete \n at the end of the line in the did change document event, if that's true, this seems to me like a plausible solution.

It's possible I am overlooking something, in that case this may be just a hot fix ignoring the actual cause.

@kraigher
Copy link
Member

kraigher commented Sep 8, 2023

I still do not understand if it is a bug in the server or the client that sends incorrect change messages. Is it possible to reproduce the problem in a unit test? That would help understand the scenario.

@Rutherther
Copy link
Contributor Author

To be honest, I am not sure about that either. What do you think the unit test should put as the change? Removal of \n at the end of a line? - That is what it looks to me the client sends. I think it would be for the best to just log the changes to a file, and I will then send this file to the linked issue. From that it should be more clear what is happening.

Even if this were a bug on the client, I would still think that since it's necessary for all lines to end with \n, it should be added anyway when it's removed, or do you think otherwise? - Of course if there is another issue causing this, that should be solved as well.

@kraigher
Copy link
Member

kraigher commented Sep 8, 2023

I think the unit test makes it a lot easier to know why the code needs to change.
It is easier for me to review the unit test than to check the youtube videos.
It also ensures the "corner case" will work in the future.
The files contents.rs you changed contains lots of unit test for the change mechanism to it should be easy to add one more.

I suspect the issue is that the client sends a change interval that is "to the right of" the end of line.
To delete a line I think the end interval must be the next line.
So a reproducing unit test would send a change that goes beyond the line it affects without going to the next line.

@Rutherther
Copy link
Contributor Author

I suspect the issue is that the client sends a change interval that is "to the right of" the end of line.
To delete a line I think the end interval must be the next line.

Yeah, I think that is the case, thanks. I will make the unit test soon.

@kraigher
Copy link
Member

@Rutherther any update on the unit test?

@Rutherther
Copy link
Contributor Author

I am so sorry about that, I had and have other stuff on my mind and didn't get around to that. I will probably not be able to get to that soon

@kraigher
Copy link
Member

kraigher commented Nov 7, 2023

Well close this for now. You can always re-open it when you have time to finish it.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Emacs strange Unexpected EOF
2 participants