-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Conversation
I'm really curious about this support - are you really building a neovim / cursorless integration? That would be cool! |
@saidelike lmk when you want me to take a preliminary look; looks like you've been busy! 🙌 |
I have updated the list of repos in the initial message. |
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!) |
@pokey A few moments later... This is ready for review :) |
There was a problem hiding this 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
packages/cursorless-engine/src/core/updateSelections/RangeUpdater.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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
packages/cursorless-neovim-e2e/src/suite/recorded.neovim.test.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-neovim-e2e/src/suite/recorded.neovim.test.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-neovim-e2e/src/suite/recorded.neovim.test.ts
Outdated
Show resolved
Hide resolved
@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 |
Just saw the gifs and was curious if the |
Of course feel free to do so. |
Yes it is a known bug. More info in #2322 |
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:
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 |
There was a problem hiding this 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
Some stats for this PR:
|
@fidgetingbits just a heads up we squashed this because the graph had gotten out of control 😅 |
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 |
Ok I'm happy with deploy now. Should we exclude the |
when I issue a "pre" command, I end up in visual mode. Is that intended? |
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. |
yep that works for me on mac! pushed |
We can probably improve the changes of mode for lots of commands. Tracked in #2453 |
Yes we can exclude I'll let you decide if you want to keep |
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 |
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! |
I actually added that precisely for deployment; that way if someone stumbles across the deployed repo they can still find their way to contribute |
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 |
once those lint warnings are fixed I'm happy to merge! |
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. |
OK I will fix these now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LEEROY JENNNKINS!
![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]>
hmmm me staring at https://github.com/hands-free-vim/cursorless.nvim but nothing happening :D |
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.
vs packages\cursorless-vscode\src\ide\vscode\VscodeClipboard.ts