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

Cursorless in neovim / terminal #2256

Merged
merged 114 commits into from
Jul 26, 2024
Merged

Conversation

saidelike
Copy link
Collaborator

@saidelike saidelike commented Feb 27, 2024

neovim_take
neovim_clone_cut_post_drink
neovim_terminal

Repositories

This currently relies on:

Todo

Checklist for pokey

The below list can be useful to review the code since some files are based on vscode similar files.

  • packages\cursorless-neovim-e2e\src\suite\recorded.neovim.test.ts versus packages\cursorless-vscode-e2e\src\suite\recorded.vscode.test.ts
  • packages\cursorless-neovim-e2e\src\endToEndTestSetup.ts versus packages\cursorless-vscode-e2e\src\endToEndTestSetup.ts
  • packages\cursorless-neovim\src\constructTestHelpers.ts versus packages\cursorless-vscode\src\constructTestHelpers.ts
  • packages\cursorless-neovim\src\extension.ts versus packages\cursorless-vscode\src\extension.ts
  • packages/cursorless-neovim/src/NeovimCommandServerApi.ts versus https://github.com/pokey/command-server/blob/main/src/extension.ts#L32
  • packages/cursorless-neovim/src/registerCommands.ts versus packages/cursorless-vscode/src/registerCommands.ts
  • packages\neovim-common\src\TestHelpers.ts versus packages\vscode-common\src\TestHelpers.ts
  • packages\neovim-common\src\getExtensionApi.ts versus packages\vscode-common\src\getExtensionApi.ts
  • packages\neovim-common\src\ide\neovim\NeovimCapabilities.ts vs packages\cursorless-vscode\src\ide\vscode\VscodeCapabilities.ts
  • packages/neovim-common/src/ide/neovim/NeovimClipboard.ts
    vs packages\cursorless-vscode\src\ide\vscode\VscodeClipboard.ts
  • packages\neovim-common\src\ide\neovim\NeovimEdit.ts vs packages\cursorless-vscode\src\ide\vscode\VscodeEdit.ts
  • packages\neovim-common\src\ide\neovim\NeovimEvents.ts vs packages\cursorless-vscode\src\ide\vscode\VscodeEvents.ts
  • packages\neovim-common\src\ide\neovim\NeovimFileSystem.ts vs packages\cursorless-vscode\src\ide\vscode\VscodeFileSystem.ts
  • packages\neovim-common\src\ide\neovim\NeovimGlobalState.ts vs packages\cursorless-vscode\src\ide\vscode\VscodeGlobalState.ts
  • packages\neovim-common\src\ide\neovim\NeovimIDE.ts vs packages\cursorless-vscode\src\ide\vscode\VscodeIDE.ts
  • packages\neovim-common\src\ide\neovim\NeovimMessages.ts vs packages\cursorless-vscode\src\ide\vscode\VscodeMessages.ts
  • packages\neovim-common\src\ide\neovim\NeovimTextDocumentImpl.ts vs packages\cursorless-vscode\src\ide\vscode\VscodeTextDocumentImpl.ts
  • packages\neovim-common\src\ide\neovim\NeovimTextEditorImpl.ts vs packages\cursorless-vscode\src\ide\vscode\VscodeTextEditorImpl.ts
  • packages\neovim-common\src\ide\neovim\NeovimTextLineImpl.ts vs packages\cursorless-vscode\src\ide\vscode\VscodeTextLineImpl.ts
  • packages\neovim-common\src\ide\neovim\hats\NeovimHats.ts vs
  • packages\neovim-common\src{neovimApi,neovimHelpers}.ts vs https://code.visualstudio.com/api/references/vscode-api
  • packages\neovim-common\src\runCommand.ts vs packages\vscode-common\src\runCommand.ts
  • packages\neovim-common\src\testUtil\openNewEditor.ts vs packages\vscode-common\src\testUtil\openNewEditor.ts
  • packages\test-harness\src\index.ts vs packages\test-harness\src\runners\extensionTestsVscode.ts
  • packages/test-harness/src/launchNeovimAndRunTests.ts vs packages/test-harness/src/launchVscodeAndRunTests.ts
  • packages/test-harness/src/scripts/runNeovimTestsCI.ts vs packages/test-harness/src/scripts/runVscodeTestsCI.ts
  • docs\contributing\cursorless-in-neovim.md vs docs\contributing\CONTRIBUTING.md

@pokey pokey marked this pull request as draft February 27, 2024 18:49
@jaresty
Copy link
Contributor

jaresty commented Mar 1, 2024

I'm really curious about this support - are you really building a neovim / cursorless integration? That would be cool!

@pokey
Copy link
Member

pokey commented Mar 11, 2024

@saidelike lmk when you want me to take a preliminary look; looks like you've been busy! 🙌

@saidelike
Copy link
Collaborator Author

I have updated the list of repos in the initial message.

@C-Loftus
Copy link
Contributor

Super excited about this. If you need testers please ping me! Would love to see screenshots/gifs of how things are going, no matter what stage that might be (no pressure though!)

@saidelike
Copy link
Collaborator Author

@saidelike lmk when you want me to take a preliminary look; looks like you've been busy! 🙌

@pokey A few moments later... This is ready for review :)

@saidelike saidelike marked this pull request as ready for review May 3, 2024 21:15
Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

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

Ok I did some initial review; I still have to look through the rest of the PR but figured I'd leave these comments for now

.github/workflows/deploy.yaml Outdated Show resolved Hide resolved
.vscode/launch.json Outdated Show resolved Hide resolved
.vscode/tasks.json Outdated Show resolved Hide resolved
.vscode/tasks.json Outdated Show resolved Hide resolved
.vscode/tasks.json Outdated Show resolved Hide resolved
cursorless.nvim/lua/cursorless/init.lua Outdated Show resolved Hide resolved
cursorless.nvim/lua/cursorless/init.lua Show resolved Hide resolved
cursorless.nvim/lua/cursorless/init.lua Outdated Show resolved Hide resolved
data/fixtures/recorded/actions/copySecondToken.yml Outdated Show resolved Hide resolved
@saidelike saidelike mentioned this pull request May 14, 2024
Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

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

Left a few more comments

.github/workflows/pre-commit.yml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
.stylua.toml Outdated Show resolved Hide resolved
.vscode/extensions.json Show resolved Hide resolved
.github/workflows/test.yml Show resolved Hide resolved
@pokey
Copy link
Member

pokey commented May 22, 2024

@saidelike would you be opposed to squashing this into one commit? I was going to pull it but that's a lot of commits 😅. Happy to help if you're not sure how to do that

@fidgetingbits
Copy link
Collaborator

Just saw the gifs and was curious if the take second paint example in the first one has a bug? It looks like it also highlights the space after the actual token? I haven't quite got it setup yet to test, so just asking so I don't forget.

@saidelike
Copy link
Collaborator Author

@saidelike would you be opposed to squashing this into one commit? I was going to pull it but that's a lot of commits 😅. Happy to help if you're not sure how to do that

Of course feel free to do so.

@saidelike
Copy link
Collaborator Author

Just saw the gifs and was curious if the take second paint example in the first one has a bug? It looks like it also highlights the space after the actual token? I haven't quite got it setup yet to test, so just asking so I don't forget.

Yes it is a known bug. More info in #2322

@pokey
Copy link
Member

pokey commented Jun 3, 2024

Ok I tried to get this working, and after fixing a couple things and running into hands-free-vim/neovim-talon#1 (comment), I eventually got stuck on this error message:

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "talon/scripting/speech_system.py", line 582, in mimic
  File "talon/scripting/speech_system.py", line 442, in engine_event
talon.engines.EngineError: failed to parse phrase: ['chuck', 'first', 'paint']
>>> sleep(1); mimic("chuck first paint")
Traceback (most recent call last):
  File "talon/scripting/talon_script.py", line 721, in run
  File "talon/scripting/talon_script.py", line 314, in run
  File "talon/scripting/actions.py", line 88, in __call__
  File "/Users/pokey/.talon/user/cursorless-talon/src/actions/actions.py", line 137, in private_cursorless_action_or_ide_command
    actions.user.cursorless_command(value, target)
  File "talon/scripting/actions.py", line 88, in __call__
  File "/Users/pokey/.talon/user/cursorless-talon/src/actions/actions.py", line 108, in cursorless_command
    actions.user.private_cursorless_command_and_wait(action)
  File "talon/scripting/actions.py", line 88, in __call__
  File "/Users/pokey/.talon/user/cursorless-talon/src/command.py", line 36, in private_cursorless_command_and_wait
    response = actions.user.private_cursorless_run_rpc_command_get(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "talon/scripting/actions.py", line 88, in __call__
  File "/Users/pokey/.talon/user/cursorless-talon/src/cursorless_command_server.py", line 33, in private_cursorless_run_rpc_command_get
    return actions.user.run_rpc_command_get(command_id, arg1, arg2)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "talon/scripting/actions.py", line 88, in __call__
  File "/Users/pokey/.talon/user/pokey_talon/apps/vscode/command_client/command_client.py", line 326, in run_rpc_command_get
    return run_command(
           ^^^^^^^^^^^^
  File "/Users/pokey/.talon/user/pokey_talon/apps/vscode/command_client/command_client.py", line 145, in run_command
    raise Exception("Must use command-server extension for advanced commands")
Exception: Must use command-server extension for advanced commands

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "talon/scripting/speech_system.py", line 582, in mimic
  File "talon/scripting/speech_system.py", line 516, in engine_event
  File "talon/scripting/executor.py", line 112, in run_phrase
  File "talon/scripting/talon_script.py", line 823, in run
  File "talon/scripting/talon_script.py", line 725, in run
talon.scripting.talon_script.TalonScriptError: 
 in script at /Users/pokey/.talon/user/cursorless-talon/src/cursorless.talon:7:
 > user.private_cursorless_action_or_ide_command(cursorless_action_or_ide_command, cursorless_target)
Exception: Must use command-server extension for advanced commands

For some reason it looks like neovim isn't actually running the command server. Maybe I missed a step, or there's a step missing in the installation instructions? Or possibly it's not running either cursorless or command-server; not sure how to verify that either are running

Copy link
Collaborator

@fidgetingbits fidgetingbits left a comment

Choose a reason for hiding this comment

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

Amazing work @saidelike ! :D Crazy how much is in here. I didn't review the .ts files closely, but still made a few comments/suggestions where I could

.pre-commit-config.yaml Outdated Show resolved Hide resolved
.vscode/launch.json Outdated Show resolved Hide resolved
.vscode/tasks.json Outdated Show resolved Hide resolved
cursorless.nvim/README.md Outdated Show resolved Hide resolved
cursorless.nvim/lua/cursorless/init.lua Show resolved Hide resolved
packages/test-harness/src/config/init.lua Outdated Show resolved Hide resolved
packages/test-harness/src/config/init.vim Outdated Show resolved Hide resolved
scripts/deploy-cursorless-nvim.sh Show resolved Hide resolved
scripts/install-neovim-dependencies.sh Outdated Show resolved Hide resolved
cursorless.nvim/lua/cursorless/cursorless.lua Show resolved Hide resolved
cursorless.nvim/README.md Outdated Show resolved Hide resolved
cursorless.nvim/README.md Outdated Show resolved Hide resolved
docs/contributing/cursorless-in-neovim.md Outdated Show resolved Hide resolved
docs/contributing/cursorless-in-neovim.md Outdated Show resolved Hide resolved
@saidelike
Copy link
Collaborator Author

saidelike commented Jun 10, 2024

Some stats for this PR:

> YAML                             +281 lines
> TypeScript                      +2905 lines
> Markdown                         +205 lines
> JSON                             +382 lines
> Shell                            +146 lines
> Lua                              +300 lines

cursorless.nvim/lua/cursorless/cursorless.lua Outdated Show resolved Hide resolved
@pokey
Copy link
Member

pokey commented Jun 13, 2024

@fidgetingbits just a heads up we squashed this because the graph had gotten out of control 😅

@pokey
Copy link
Member

pokey commented Jul 26, 2024

I haven't been able to get cursorless in terminal to work on my Mac at all today so can't sanely confirm shortcuts actually work, but it seems like maybe ctrl-alt-\ and <C-M-> works on default Mac terminal in so far as I see similar failure as ctrl-q, so dunno if you want to try that @pokey since your setup works at least. That shortcut doesn't work for me on Linux, but I think that's okay since `ctrl-`` is working on Linux.

ok i'll give that a whirl. any idea why cursorless in terminal isn't working for you on mac? was working fine for me last I tried

@pokey
Copy link
Member

pokey commented Jul 26, 2024

Ok I'm happy with deploy now. Should we exclude the test directory?

@pokey
Copy link
Member

pokey commented Jul 26, 2024

when I issue a "pre" command, I end up in visual mode. Is that intended?

@fidgetingbits
Copy link
Collaborator

I haven't been able to get cursorless in terminal to work on my Mac at all today so can't sanely confirm shortcuts actually work, but it seems like maybe ctrl-alt-\ and <C-M-> works on default Mac terminal in so far as I see similar failure as ctrl-q, so dunno if you want to try that @pokey since your setup works at least. That shortcut doesn't work for me on Linux, but I think that's okay since `ctrl-`` is working on Linux.

ok i'll give that a whirl. any idea why cursorless in terminal isn't working for you on mac? was working fine for me last I tried

Not sure yet. But it always times out waiting for the command server to respond. My mac is mostly all built/setup with nix (nix-darwin) so I'm guessing it's something I fixed on nixos and forgot to document. Guessing it won't be something that happens on a normal macos system.

@pokey
Copy link
Member

pokey commented Jul 26, 2024

maybe ctrl-alt-\ and <C-M-> works on default Mac terminal in so far as I see similar failure as ctrl-q, so dunno if you want to try that @pokey since your setup works at least. That shortcut doesn't work for me on Linux, but I think that's okay since `ctrl-`` is working on Linux.

yep that works for me on mac! pushed

@saidelike
Copy link
Collaborator Author

when I issue a "pre" command, I end up in visual mode. Is that intended?

We can probably improve the changes of mode for lots of commands. Tracked in #2453

@saidelike
Copy link
Collaborator Author

Ok I'm happy with deploy now. Should we exclude the test directory?

Yes we can exclude test/, .busted.

I'll let you decide if you want to keep CONTRIBUTING.md but I guess we want it in the production repo so people can read about it.

@pokey
Copy link
Member

pokey commented Jul 26, 2024

there is tons of commented-out code in the neovim packages, but I'm trying not to look and just let you guys own it 🙈😅

might be worth filing a follow-up to clean it up if it bothers you as well, but again, I'm trying to treat these packages as if I don't own them and let you guys have some flexibility, so do what you want

@pokey
Copy link
Member

pokey commented Jul 26, 2024

ok this PR is ready to go from my perspective. I'm going to do one final check where I push something broken and make sure CI fails. If that happens I'll file a revert commit and let's ship it!

@pokey
Copy link
Member

pokey commented Jul 26, 2024

I'll let you decide if you want to keep CONTRIBUTING.md but I guess we want it in the production repo so people can read about it.

I actually added that precisely for deployment; that way if someone stumbles across the deployed repo they can still find their way to contribute

@pokey
Copy link
Member

pokey commented Jul 26, 2024

ok the last issue I found is that there are a bunch of eslint warnings https://github.com/cursorless-dev/cursorless/actions/runs/10110660353/job/27961014181?pr=2256#step:9:103

I'm going to turn them into errors in #2580, so we should fix those up before merging. Remember that for unused args, you can prepend them with _

@pokey
Copy link
Member

pokey commented Jul 26, 2024

once those lint warnings are fixed I'm happy to merge!

@saidelike
Copy link
Collaborator Author

there is tons of commented-out code in the neovim packages, but I'm trying not to look and just let you guys own it 🙈😅

might be worth filing a follow-up to clean it up if it bothers you as well, but again, I'm trying to treat these packages as if I don't own them and let you guys have some flexibility, so do what you want

Yeah happy to do that cleanup in a follow-up PR. I've created #2582. Feel free to add any other point you think is worth doing.

@saidelike
Copy link
Collaborator Author

ok the last issue I found is that there are a bunch of eslint warnings https://github.com/cursorless-dev/cursorless/actions/runs/10110660353/job/27961014181?pr=2256#step:9:103

I'm going to turn them into errors in #2580, so we should fix those up before merging. Remember that for unused args, you can prepend them with _

OK I will fix these now

@saidelike
Copy link
Collaborator Author

Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

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

LEEROY JENNNKINS!

@pokey pokey enabled auto-merge July 26, 2024 12:41
@pokey pokey added this pull request to the merge queue Jul 26, 2024
github-merge-queue bot pushed a commit that referenced this pull request Jul 26, 2024
![neovim_take](https://github.com/cursorless-dev/cursorless/assets/387346/e72acd0d-fee2-4bae-a8b1-0cf8644c0ecf)

![neovim_clone_cut_post_drink](https://github.com/cursorless-dev/cursorless/assets/387346/20041f0d-8ff1-41b8-a2bd-02bc353ad8c5)

![neovim_terminal](https://github.com/cursorless-dev/cursorless/assets/387346/423b6d29-a1e4-4910-8a4e-32acd5dd3c5b)

# Repositories

This currently relies on:
* https://github.com/saidelike/cursorless/tree/nvim-talon (this PR) 
* compiled and pushed to
https://github.com/hands-free-vim/cursorless.nvim (neovim cursorless
plugin)
* https://github.com/saidelike/command-server/tree/neovim
  * compiled and pushed to cursorless mono repo
* https://github.com/hands-free-vim/talon.nvim (neovim talon plugin)
* https://github.com/hands-free-vim/neovim-talon (talon commands for
neovim: command-client, commands for navigating/editing/split/tabs in
editor. Deprecates https://github.com/fidgetingbits/talon-vim)

# Todo

- [x] hands-free-vim/neovim-talon#20
- [x] hands-free-vim/neovim-talon#24

# Checklist for pokey

The below list can be useful to review the code since some files are
based on vscode similar files.

- packages\cursorless-neovim-e2e\src\suite\recorded.neovim.test.ts
versus packages\cursorless-vscode-e2e\src\suite\recorded.vscode.test.ts
- packages\cursorless-neovim-e2e\src\endToEndTestSetup.ts versus
packages\cursorless-vscode-e2e\src\endToEndTestSetup.ts
- packages\cursorless-neovim\src\constructTestHelpers.ts versus
packages\cursorless-vscode\src\constructTestHelpers.ts
- packages\cursorless-neovim\src\extension.ts versus
packages\cursorless-vscode\src\extension.ts
- packages/cursorless-neovim/src/NeovimCommandServerApi.ts versus
https://github.com/pokey/command-server/blob/main/src/extension.ts#L32
- packages/cursorless-neovim/src/registerCommands.ts versus
packages/cursorless-vscode/src/registerCommands.ts
- packages\neovim-common\src\TestHelpers.ts versus
packages\vscode-common\src\TestHelpers.ts
- packages\neovim-common\src\getExtensionApi.ts versus
packages\vscode-common\src\getExtensionApi.ts
- packages\neovim-common\src\ide\neovim\NeovimCapabilities.ts vs
packages\cursorless-vscode\src\ide\vscode\VscodeCapabilities.ts
- packages/neovim-common/src/ide/neovim/NeovimClipboard.ts
 vs packages\cursorless-vscode\src\ide\vscode\VscodeClipboard.ts
- packages\neovim-common\src\ide\neovim\NeovimEdit.ts vs
packages\cursorless-vscode\src\ide\vscode\VscodeEdit.ts
- packages\neovim-common\src\ide\neovim\NeovimEvents.ts vs
packages\cursorless-vscode\src\ide\vscode\VscodeEvents.ts
- packages\neovim-common\src\ide\neovim\NeovimFileSystem.ts vs
packages\cursorless-vscode\src\ide\vscode\VscodeFileSystem.ts
- packages\neovim-common\src\ide\neovim\NeovimGlobalState.ts vs
packages\cursorless-vscode\src\ide\vscode\VscodeGlobalState.ts
- packages\neovim-common\src\ide\neovim\NeovimIDE.ts vs
packages\cursorless-vscode\src\ide\vscode\VscodeIDE.ts
- packages\neovim-common\src\ide\neovim\NeovimMessages.ts vs
packages\cursorless-vscode\src\ide\vscode\VscodeMessages.ts
- packages\neovim-common\src\ide\neovim\NeovimTextDocumentImpl.ts vs
packages\cursorless-vscode\src\ide\vscode\VscodeTextDocumentImpl.ts
- packages\neovim-common\src\ide\neovim\NeovimTextEditorImpl.ts vs
packages\cursorless-vscode\src\ide\vscode\VscodeTextEditorImpl.ts
- packages\neovim-common\src\ide\neovim\NeovimTextLineImpl.ts vs
packages\cursorless-vscode\src\ide\vscode\VscodeTextLineImpl.ts
- packages\neovim-common\src\ide\neovim\hats\NeovimHats.ts vs 
- packages\neovim-common\src\{neovimApi,neovimHelpers}.ts vs
https://code.visualstudio.com/api/references/vscode-api
- packages\neovim-common\src\runCommand.ts vs
packages\vscode-common\src\runCommand.ts
- packages\neovim-common\src\testUtil\openNewEditor.ts vs
packages\vscode-common\src\testUtil\openNewEditor.ts
- packages\test-harness\src\index.ts vs
packages\test-harness\src\runners\extensionTestsVscode.ts
- packages/test-harness/src/launchNeovimAndRunTests.ts vs
packages/test-harness/src/launchVscodeAndRunTests.ts
- packages/test-harness/src/scripts/runNeovimTestsCI.ts vs
packages/test-harness/src/scripts/runVscodeTestsCI.ts
- docs\contributing\cursorless-in-neovim.md vs
docs\contributing\CONTRIBUTING.md

---------

Co-authored-by: Cedric Halbronn <[email protected]>
Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
Co-authored-by: fidgetingbits <[email protected]>
Co-authored-by: Pokey Rule <[email protected]>
Merged via the queue into cursorless-dev:main with commit 0ec5e3c Jul 26, 2024
15 checks passed
@pokey pokey deleted the nvim-talon branch July 26, 2024 13:00
@saidelike
Copy link
Collaborator Author

hmmm me staring at https://github.com/hands-free-vim/cursorless.nvim but nothing happening :D

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 this pull request may close these issues.

7 participants