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

Goals don't update on cursor movement when using VsCodeVim #163

Closed
jesyspa opened this issue Jan 9, 2023 · 4 comments · Fixed by #170 or ocaml/opam-repository#22956
Closed

Goals don't update on cursor movement when using VsCodeVim #163

jesyspa opened this issue Jan 9, 2023 · 4 comments · Fixed by #170 or ocaml/opam-repository#22956
Labels
kind: bug Something isn't working part: client (VSCode)
Milestone

Comments

@jesyspa
Copy link

jesyspa commented Jan 9, 2023

As discussed on Zulip, in version 0.1.2 of coq-lsp, when show_goals_on is set to "Show on click and cursor movement" and the VsCodeVim extension is enabled, moving the cursor using hjkl or the arrow keys doesn't update the goal status (and in fact there's no activity in Coq LSP Server Events).

@ejgallego ejgallego added the kind: bug Something isn't working label Jan 9, 2023
@ejgallego
Copy link
Owner

ejgallego commented Jan 9, 2023

Indeed, it seems that the problem is that VsCodeVim doesn't call window.onDidChangeTextEditorSelection which is what coq-lsp uses to keep track of the cursor.

@jesyspa maybe we could ask the VsCodeVim developers what's the correct way to track cursor movement when their extension is enabled?

I bet they use their own selection implementation which indeed makes sense for something like a VIM emulator.

This way, we could detect that and hook to the correct method.

@jesyspa
Copy link
Author

jesyspa commented Jan 9, 2023

I don't have any experience with writing VS Code plugins so I may be misunderstanding: should it be on each plugin to notify when the text editor selection changes, or is this a VSCode issue? I tested it with vscode-neovim and the behaviour is the same...

It seems like for normal movement, VsCodeVim uses the cursorMove built-in command, though I may be misreading it. I can't find anything in the documentation about whether cursorMove should call onDidChangeTextEditorSelection (or anything else). I'm looking to see whether there's any good way to get a log of all raised events to see what gets triggered...

@jesyspa
Copy link
Author

jesyspa commented Jan 9, 2023

A little bit of testing later: the events are being generated, but they have type Command instead of type Keyboard, hence why they're being ignored. I'm not sure why this would be the case: I can't see anything in the options of cursorMove that would determine one or the other...

I think the simplest solution may be to add a corresponding option for show_goals_on, or treat Command as Keyboard. What do you think?

Here's the code of the extension I cooked up to test this: https://github.com/jesyspa/jytest-vscode

@ejgallego
Copy link
Owner

Oh, that's a great finding @jesyspa , I think indeed with that info we can fix your problem.

Indeed I wonder if there is a problem in identifying Command with Keyboard, not sure if some extensions can use that programatically and hence create a bit of chaos in terms of goal display, let's have a look at the docs.

@ejgallego ejgallego added this to the 0.1.3 milestone Jan 10, 2023
ejgallego added a commit that referenced this issue Jan 11, 2023
Fixes #163

This makes VsCodeVim cursor tracking work; thanks to Cactus (Anton)
Golov for detailed bug reporting and testing.
ejgallego added a commit to ejgallego/opam-repository that referenced this issue Jan 15, 2023
CHANGES:

----------------------

 - Much improved handling of Coq fatal errors, the server is now
   hardened against them (@ejgallego, ejgallego/coq-lsp#155, ejgallego/coq-lsp#157, ejgallego/coq-lsp#160, fixes ejgallego/coq-lsp#91)
 - `coq-lsp` now follows the LSP specification regarding
   initialization strictly (@ejgallego, ejgallego/coq-lsp#168)
 - New setting for goals to be updated when the selection changes due
   to a command; this makes VsCodeVim cursor tracking work; thanks to
   Cactus (Anton) Golov for detailed bug reporting and testing
   (@ejgallego, @jesyspa, ejgallego/coq-lsp#170, fixes ejgallego/coq-lsp#163)
 - `coq-lsp` will now warn the user when two files have been opened
   simultaneously and the parser may go into a broken state :/
   (@ejgallego, ejgallego/coq-lsp#169)
 - Implement request postponement and cancellation. Thus
   `documentSymbols` will now be postponed until the document is
   ready, (@ejgallego, ejgallego/coq-lsp#141, ejgallego/coq-lsp#146, fixes ejgallego/coq-lsp#124)
 - Protocol and VS Code interfaces now support shelved and given_up
   goals (@ejgallego, ejgallego/coq-lsp#175)
 - Allow to postpone requests to wait for data to become available on
   document points; this is implemented to provide a nicer "show goals
   while I type" experience. Client default has been changed to "show
   goals on mouse, click, typing, and cursor movement) (@ejgallego,
   ejgallego/coq-lsp#177, ejgallego/coq-lsp#179)
 - Store stats per document (@ejgallego, ejgallego/coq-lsp#180, fixes ejgallego/coq-lsp#173)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working part: client (VSCode)
Projects
None yet
2 participants