-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: Add tasks for debugging neovim on linux #8
Conversation
This now adds other changes I needed to make the tests actually run in my nix dev machine, but with these changes it mostly works. I still run into cursorless-dev#2446 and cursorless-dev#2445 though. I add the flake.nix for now, with changes that build upon cursorless-dev#1908. Atm cursorless-neovim requires the latest neovim node-client, which isn't in nixpkgs yet. I have an open PR for that, but in the meantime I have to do a manual override from my PR branch, thus the big overlay, which will eventually go away. I also explicitly call |
Sounds good to me. A few minor feedbacks and rebasing to latest nvim-talon should fix the test on CI failure and we can merge. @fidgetingbits cursorless-dev#1908 has been merged in main (hence in nvim-talon branch) so I am guessing all the nix stuff can be removed from this PR and it will avoid the merge conflicts too? |
will rebase later today hopefully and see ya. not quite all the Nix stuff can be removed. my PR to update the neovim node plugin in nixpkgs still hasn't landed, so the overlay will still be needed, but the diff will be simpler after rebase for sure. |
1cdf49e
to
d124747
Compare
Just finished locally testing this after rebase and is all good. Ran into a weird issue related to one of pokey's changes where it couldn't Just have to wait for the windows CI test to pass, but it seems like the other ones are passing now so hopefully is good. |
Ready to be merged, cc @pokey |
This should close cursorless-dev#2366, as @pokey added OS X tests already. We can't assume what terminal exists on the linux development machine so I added a way to invoke a bunch of the common ones. The caveat here is that you might end up having multiple terminals installed and it could run a terminal that you prefer isn't the default, but I don't think that's a huge deal atm.
There's obviously lots of other terminals that I didn't add but I think this is good enough for now and we can add more in the unlikely event other developers crop up that run into others.
I haven't got as far as being able to actually run the tests yet, but that's due to a bug I need to hunt down that's separate from these tasks. These both at least run and allow me to debug them on attach.
Checklist