Skip to content
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

Text edits which have the same start position are reversed #281

Closed
kyoh86 opened this issue Jul 6, 2024 · 9 comments
Closed

Text edits which have the same start position are reversed #281

kyoh86 opened this issue Jul 6, 2024 · 9 comments

Comments

@kyoh86
Copy link
Contributor

kyoh86 commented Jul 6, 2024

In LSP spec, an order of the text-edits which have the same start position should be applied in order of them.
Neovim LSP client was applied them in reversed order, but it's fixed in neovim/neovim#29202.
So now, formatting some continuous lines with efm-langserver, text lines are reversed.
Please see the issue neovim/neovim#29392 for steps to reproduce and details.

kyoh86 added a commit to kyoh86/efm-langserver that referenced this issue Jul 12, 2024
@kyoh86
Copy link
Contributor Author

kyoh86 commented Jul 12, 2024

In the golang/tools, the ComputeEdits is being used only as testing util....
https://github.com/golang/tools/blob/d9c6af3f039a6193d1557d780752f223b3978173/internal/diff/myers/diff.go

@kyoh86
Copy link
Contributor Author

kyoh86 commented Jul 13, 2024

It is deprecated in the golang/tools.
I think it would be better to re-implement using google/go-cmp or something similar.

@kyoh86
Copy link
Contributor Author

kyoh86 commented Jul 13, 2024

The reason for the deprecated status was not related to this project, so I think that we can continue to use the ComputeEdits as it is.

@kyoh86
Copy link
Contributor Author

kyoh86 commented Jul 14, 2024

#282 (comment)

I had neglected to mention that this change would affect other clients as well.
At a minimum I would check for the following clients

  • vim-lsp
  • coc.nvim
  • Eglot
  • Helix
  • VSCode

Since it will take time to verify all environments, we will withdraw the request and ask for another review after the investigation.

If anybody could help me for testing them, give me your help.

@conao3
Copy link

conao3 commented Jul 15, 2024

I will try to check this issue using Eglot.

@kyoh86
Copy link
Contributor Author

kyoh86 commented Jul 15, 2024

📝 I finished testing for vim-lsp, helix and coc.nvim.

@conao3
Copy link

conao3 commented Jul 15, 2024

I checked Eglot. It worked the same way as vim-lsp.

@konradmalik
Copy link

I believe that the TL;DR of this is that the fix recently introduced in neovim was not a fix, but a regression in fact.
This PR reverted the previous change, added more tests and fixed the behavior: neovim/neovim#29877

@kyoh86
Copy link
Contributor Author

kyoh86 commented Jul 29, 2024

yes, it has fixed in Neovim!

@kyoh86 kyoh86 closed this as completed Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants