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

feat(keybindings): execute keybinding starting with : as ex command #2397

Merged
merged 3 commits into from
Sep 7, 2020

Conversation

glennsl
Copy link
Member

@glennsl glennsl commented Sep 3, 2020

A simple change that allows keybindings to invoke ex commands directly, and could address quite a few of the requests in #1423. The downside of doing this instead of implementing proper commands is of course that they won't be discoverable through the command palette, but this doesn't prevent proper commands to be added either, so I think it might be a nice convenient stop-gap solution to unblock people at least.

Example:

  {key: "kk", command: ":split", when: "editorTextFocus"}

Let me know if you think this is a viable approach @bryphe, then I'll add a few integration tests before it's merged.

Fixes #807

@glennsl glennsl added the WIP label Sep 3, 2020
@glennsl glennsl requested a review from bryphe September 3, 2020 12:14
@bryphe
Copy link
Member

bryphe commented Sep 3, 2020

Thanks for implementing this, @glennsl ! Approach looks great to me.

I think it might be a nice convenient stop-gap solution to unblock people at least.

I agree - and definitely fits in to the 0.6.0 milestone work. 👍

This fixes #807 as well

@glennsl
Copy link
Member Author

glennsl commented Sep 4, 2020

Oh wow, exactly what you proposed. And here I was thinking I had novel ideas! Probably had that one stored away in the subconscious somewhere :D I'll add some integration tests then.

@glennsl
Copy link
Member Author

glennsl commented Sep 4, 2020

I'm unfortunately not able to run any integration tests locally because I get a sanitation error:

==63020==ERROR: AddressSanitizer failed to deallocate 0x2000 (8192) bytes at address 0x6250001e0100
==63020==AddressSanitizer CHECK failed: /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_posix.cpp:61 "(("unable to unmap" && 0)) != (0)" (0x0, 0x0)

So I'm just pushing and hoping they'll work...

@glennsl
Copy link
Member Author

glennsl commented Sep 4, 2020

/azp run

@glennsl glennsl removed the WIP label Sep 4, 2020
@glennsl
Copy link
Member Author

glennsl commented Sep 4, 2020

/azp run

2 similar comments
@glennsl
Copy link
Member Author

glennsl commented Sep 4, 2020

/azp run

@glennsl
Copy link
Member Author

glennsl commented Sep 5, 2020

/azp run

@glennsl
Copy link
Member Author

glennsl commented Sep 5, 2020

CI is being very difficult, but it's just Validate Windows Release that times out now.

@bryphe
Copy link
Member

bryphe commented Sep 7, 2020

Thanks for adding those tests, @glennsl - sorry about the CI difficulties. I bumped the Windows machine and it's green now. Looks great!

@bryphe bryphe merged commit e051efd into onivim:master Sep 7, 2020
@glennsl glennsl deleted the feat/keybindings/ex-commands branch September 7, 2020 16:59
@bryphe bryphe added the B-license-key Bounty: License Key label Sep 7, 2020
@bryphe
Copy link
Member

bryphe commented Sep 7, 2020

Oh, and with license key bounty - this would definitely be a qualifier for it. LMK if you want an extra license key, @glennsl

@glennsl
Copy link
Member Author

glennsl commented Sep 8, 2020

Thanks! But no need to worry about bounties on my account 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-license-key Bounty: License Key
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: let an Oni2 command that sends vimscript be writable in the keybindings file
2 participants