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

Add all commands to chrome's commands API #56

Closed
wants to merge 1 commit into from

Conversation

sooqua
Copy link

@sooqua sooqua commented Jul 17, 2019

Add all commands to chrome.commands. This allows to trigger commands
with highest priority using the keyboard shortcuts defined in
chrome://extensions/shortcuts
This closes #55

Add all commands to chrome.commands. This allows to trigger commands
with highest priority using the keyboard shortcuts defined in
chrome://extensions/shortcuts
This closes gdh1995#55
@gdh1995
Copy link
Owner

gdh1995 commented Jul 17, 2019

But this might not work as expected: "most" (>= 80%) commands need real chrome.runtime.Port instances, so they are not suitable, and not needed to be global.

Added: currently Vimium C can grab most keyboard events, unless:

@sooqua
Copy link
Author

sooqua commented Jul 17, 2019

Could you clarify on ">= 80%"? I couldn't find a single command that doesn't work.
P.S. Sorry for 4-space indent =\

@gdh1995
Copy link
Owner

gdh1995 commented Jul 17, 2019

For example, all of content commands relies a port to tell a content page what to do. Of course we may call indexPorts(tabId)[0] to get the current active frame's port, but I think, "commands.onCommand->indexPorts->port.postMessage(Execute{})" causes no difference compared with just "grabKeyDown->checkKey->requestBackgroundForOptions->execute", because on pages like "chrome://*", there're no relevant ports, so no content commands can work.

The only detail as I remembed that may be different is, for an iframe of <ifream> of about:srcdoc, Chrome won't run Vimium C on it, so global commands may do help you trigger some commands, But, then there'll be some focus issues - every content command have to focus the frame it's been triggered on.

background commands

Some background commands also depend on ports, if parts of its logic wants some info on content pages, like frame title, URL and selected text. Besides, some error reports also expect ports to show on Vimium C's HUD.

This is why I didn't expose an option to re-map the 4 global commands - only quite limited comands are suitable, but I was once not interested enough to collect them. But since requested, maybe I'll add 1~3 new commands to manifest.json.

design

I think the design of chrome.commands is not for such usage that make users select 4 from tons of command candidates, and the list ought to be short enough.

@sooqua
Copy link
Author

sooqua commented Jul 17, 2019

Sorry, I'm not a good programmer and I barely ever wrote a browser extension. So I'm not the one to know and/or understand the imperfections of this implementation. What caused me to do this is that I usually want <C-*>, <M-*> etc. shortcuts to be always accessible, whether I'm in insert mode or not. I mean, that's why they have a modifier key. They're supposed to work everywhere.
So all I wanted is to have shortcuts with highest priority. Besides, what can be more life-changing than being able to bind <C-w> to "blank"? Because I keep pressing it while trying to erase a word! I had to re-write this message 3 times! And I don't need <C-w> with this extension.
So this solves it for me. Maybe there's a better way to do this. I hope you can resolve all internal implementation details and ultimately add more commands to the commands' API, if you refuse to merge this. I will have to stick to my local version until then.

@sooqua
Copy link
Author

sooqua commented Jul 17, 2019

I think the design of chrome.commands is not for such usage that make users select 4 from tons of command candidates, and the list ought to be short enough.

Users are not limited to only 4 "global" shortcuts. They can configure as many as they'd like. You are only limited to having 4 commands with the "suggested_key" attribute.

@gdh1995 gdh1995 mentioned this pull request Jul 18, 2019
@gdh1995
Copy link
Owner

gdh1995 commented Jul 28, 2019

Deprecated by Vimium C v1.76.2 which has implemented #55 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More global shortcuts?
2 participants