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

Write (:w) in HTML files selects the whole file and loses position when HTML LSP is active #3859

Closed
aral opened this issue Sep 16, 2022 · 8 comments · Fixed by #4041
Closed
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug

Comments

@aral
Copy link
Contributor

aral commented Sep 16, 2022

Summary

Issuing a :w command in an HTML file selects the whole file and loses your current position when HTML LSP is active.

Reproduction Steps

I tried this:

  1. npm i -g vscode-langservers-extracted to install HTML LSP
  2. Create HTML file
  3. :w

I expected this to happen:

File is saved. Nothing is selected. My position is saved.

Instead, this happened:

File is saved. Everything is selected. My position is lost (reverts to top).

Note that this behaviour does not occur if the LSP is not installed/active.

Helix log

~/.cache/helix/helix.log
2022-09-16T11:57:07.545 helix_term::application [ERROR] Language Server: Method not found client/r

Platform

Linux (Fedora Silverblue 36)

Terminal Emulator

Black Box

Helix Version

helix 22.08.1 (714db9c)

@aral aral added the C-bug Category: This is a bug label Sep 16, 2022
@archseer
Copy link
Member

That's because the formatting response from the LSP effectively removes the whole document then inserts the formatted document (rather than a diff)

@aral
Copy link
Contributor Author

aral commented Sep 16, 2022

So is the fix not to include the formatting feature? Because this makes it unusable.

@the-mikedavis
Copy link
Member

The LSP spec allows for the language server to say what changed in the formatting response in small diffs rather than resending the whole (potentially large) document. This is similar to #2655

See also #2752 - it may happen because of line-endings of the document. If the language server is replacing CRLFs with LFs it may send the whole document, but that should be a one-time occurrence.

@kirawi
Copy link
Member

kirawi commented Sep 16, 2022

Alternatively, you can specify an external formatter which Helix will generate a diff for.

@archseer
Copy link
Member

The solution is to detect a whole document replacement by the format action and run it through the same code we use for external formatters to generate a diff.

@kirawi
Copy link
Member

kirawi commented Sep 17, 2022

similar provides a function: https://docs.rs/similar/2.2.0/similar/fn.get_diff_ratio.html

On second thought, that sounds a little expensive.

@kirawi
Copy link
Member

kirawi commented Sep 17, 2022

Ah right, we could just convert TextEdits into DiffOps. We could generate a diff if a certain amount of the file is entirely replaced, or we could generate a diff only when the entire document is replaced (which is easier/simpler, and what I think most users encounter).

@archseer
Copy link
Member

We only want to deal with the latter and it should be easy to detect: a single change that spans the entire document range.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants