Remap refactor get leader key original action and handle plugins through mappings #4995
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This PR doesn't change much for the user except the ability to remap plugin keys and getting the original leader key action back. But under the hood this PR makes plugins easier to maintain and implement. The next step would be to find a way of implementing a This will also allow to create mappings for our implemented plugins by reading them from the user .vimrc file, which I intend to implement after this gets merged. And if in the future there is any need to create custom plugins for VSCodeVim by creating other vscode extensions that implement our |
berknam
force-pushed
the
remapRefactor_LeaderKeyOpt3
branch
from
5cdb7ae to
9fbd5a1
Compare
Travis tests have failedHey @berknam, Node.js: 12if [[ $(git diff-index HEAD -- *.js *.ts *.md) ]]; then git diff; echo "Prettier Failed. Run `gulp forceprettier` and commit changes to resolve."; exit 1; figulp buildgulp prepare-testnpm testTravisBuddy Request Identifier: 359bcce0-c1f6-11ea-8e31-7fd6e6dac888 |
berknam
force-pushed
the
remapRefactor_LeaderKeyOpt3
branch
from
9fbd5a1 to
5551ec8
Compare
Travis tests have failedHey @berknam, Node.js: 12if [[ $(git diff-index HEAD -- *.js *.ts *.md) ]]; then git diff; echo "Prettier Failed. Run `gulp forceprettier` and commit changes to resolve."; exit 1; finpm testTravisBuddy Request Identifier: a36a1f80-c1f9-11ea-8e31-7fd6e6dac888 |
berknam
force-pushed
the
remapRefactor_LeaderKeyOpt3
branch
from
0966303 to
768db4b
Compare
berknam
force-pushed
the
remapRefactor_LeaderKeyOpt3
branch
from
768db4b to
143ef4d
Compare
berknam
force-pushed
the
remapRefactor_LeaderKeyOpt3
branch
from
143ef4d to
79094cd
Compare
|
Hi @berknam
Any ideas? |
|
@sql-koala You also need to add the plugin name on the file private isPluginActive(config: IConfiguration, pluginName: string | undefined) {
if (!pluginName) {
return true;
} else {
switch (pluginName) {
case 'camelcasemotion':
return config.camelCaseMotion.enable;
case 'easymotion':
return config.easymotion;
case 'replacewithregister':
return config.replaceWithRegister;
case 'sneak':
return config.sneak;
case 'surround':
return config.surround;
// ----------------------------
// This is what you need to add:
case 'Exchange':
return config.exchange;
// ----------------------------
default:
return false;
}
}
}I really need to make this automatic as well, when registering the actions for a given plugin if it doesn't know about it yet it should create its configuration enabler check. But I'm still thinking how to do this, because it would require that all plugins are enabled/disabled the same way and currently that's not true. I could create a But for now it has to be done manually. |
|
Also if you want to be able to repeat your operator with @RegisterPluginAction('Exchange')
export class ExchangeOperator extends BaseOperator {
pluginActionDefaultKeys = ['c', 'x'];
keys = ['<Plug>CExchange'];
public modes = [Mode.Normal, Mode.OperatorPendingMode];
...
} |
|
Thanks @berknam for the quick help; that worked. |
berknam
force-pushed
the
remapRefactor_LeaderKeyOpt3
branch
from
79094cd to
68d7e57
Compare
- Implements the 'BasePluginAction' to be registered with '@RegisterPluginAction' by every plugin action - Create default keybindings when registering plugin actions - Add this default keybindings to the user keybindings when validating while also filtering out the default mappings that already have a map to made by the user - Make the NormalizeKey return the original key when there is no change so that keys like '<CamelCaseMotion_w>' aren't changed to '<camelcasemotion_w>
- Implement '@RegisterPluginActions' on camel case motion actions - Remove logic from 'CompareKeypressSequence' that handled the leader key to allow the leader key to be compared normally to itself - Make the action handler get the relevant action by looking for actions the 'vimState.currentModeIncludingPseudoModes' - Change the 'doesActionApply' and 'couldActionApply' to look get the current mode with pseudo modes as well - Add OperatorPendingMode to all actions where it applies - Add OperatorPendingMode to all operators that can run repeated, like 'dd', 'cc', 'gUU' - Add OperatorPendingMode to all textObjects and remove NormalMode as textObjects only apply on OperatorPending, Visual or VisualBlock modes
- Should have whichwrap with 'b'
other than NormalMode
- 'RegisterPluginAction' now takes the plugin name as a parameter - 'RegisterPluginAction' adds plugin name to the default keybindings it creates so that the remappingValidator can check if that plugin is active before adding the keybinding to the map of bindings
- When registering the plugin actions it will now also store the default mappings on the var 'Globals.mockConfigurationDefaultBindings' and when creating a new testConfiguration on the Tests it loads this defaults - This made it necessary to clone the default mappings before validating otherwise they would be changed by the tests messing it up for the next tests to come.
- Fix surround 'yss' not working - Add check for '<leader>' key on 'handleKeyEvent' because the plugin tests might send this key and it needs to be swapped by its representation before actually handling it. - Fix the Notation.NormalizeKey missing '>' character - Fix the Notation.NormalizeKey regex replacing some characters on the new plugin keys by mistake. Like the key '<(easymotion-bd-t)>' was being changed to '<(easymotion-bD-t)>' - Fix bug on camelCaseMotion test with the plugin disable that now handles the leader key while before it didn't
- Changed the plugin keys to be the same as used by its corresponding Vim plugin, like '<Plug>(easymoption-s)' or '<Plug>Sneak_s' - Create the default mappings only to the plugin key but allow the actions to accept more keys. For example: The sneak plugin 's' key is mapped to '<Plug>Sneak_s' but the action it self has the keys `['<Plug>Sneak_s', '<character>, '<character>']`, meaning that after the <Plug> key it still needs two characters.
- Fix remappingValidator still creating remaps for default remappings when the user already had remaps to that same remap. (Example: if the user remaps '<leader>sw <Plug>CamelCaseMotion_w' the remappingValidator should not create the default remap '<leader>w <Plug>CamelCaseMotion_w'
- Add default mappings for 'f','F','t','T' when config.sneakReplacesF is set to true and remove them when it is false - Add '<Plug>Sneak_t' and '<Plug>Sneak_T' when config.sneakReplacesF is set - Add tests for this new 'till' movements
- Create 'StartSurroundMode' function similar to the previous 'CommandSurroundModeStart' action exec - Create 'BaseSurroundCommand' used by 'cs', 'ds' and 'S' in visual mode - Implements 'ys' has an operator - Make all surround actions only apply if surround is set in configuration - Add 'surround' plugin name to the 'isPluginActive'' from remappingValidator
- Implement the following keys from the surround plugin: 'cS', 'yS' (operator), 'ySs', 'ySS' (same as 'ySs') and 'gS' (visual mode) - Now the only missing parts of surround plugin is the insert mode mappings, but those aren't that important anyway. - Still missing tests for these new actions
- There should be no need to implement a full OperatorPendingMode, even Vim doesn't do it, in vim operator pending mode is considered 'no' or 'nov', 'noV', 'no^V' mode which is normal mode with operator pending mode with optinally forced parameters
operatorPending or visual modes
berknam
force-pushed
the
remapRefactor_LeaderKeyOpt3
branch
from
68d7e57 to
e513902
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
It creates a new way of handling plugins. Now the plugin's main actions are registered through the new
@RegisterPluginAction(pluginName)which takes the newBasePluginActionwith the new propertypluginActionDefaultKeys: string[]which holds the keys that will be used to create the default remappings. Thekeysproperty now needs to have the plugin action name (the same as used by that plugin in Vim) as the first key, but can have more keys.Examples:
Plugin: CamelCaseMotion
Action: Move to "camel word" begin
In Vim you would do
map <leader>w <Plug>CamelCaseMotion_w, here we have:Which when registering will create the default remapping
<leader>w-><Plug>CamelCaseMotion_w, here we consider<Plug>CamelCaseMotion_was a single key.Plugin: Sneak
Action: find 2 chars
In Vim you would do
nmap s <Plug>Sneak_s, here we have:Here even though we have 3 keys for the action we only map the
pluginActionDefaultKeysto the first key of keys, so we get a remap ofs -> <Plug>Sneak_swhich when it gets to the action handler waits for two more keys of type <character>.This was implemented on all plugins and since I was changing them I also added some features to them (this method actually makes it easier to add more features from these plugins) like the
<Plug>Sneak_t,<Plug>Sneak_Tfrom sneak and the keys 'cS', 'yS' (operator), 'ySs', 'ySS' (same as 'ySs') and 'gS' (visual mode) from surround plugin.On register we create the default mappings but we add them to the mappings Maps when loading config depending on whether the plugin is active or not, we also don't add the plugin default mapping if the user already has a map for that key. Example: if the user has this map:
w-><Plug>CamelCaseMotion_wwe don't create the mapping for<leader>w. This is a behavior that a lot of plugins do in Vim as well.The result of all this is not only do we have a better implementation of plugins, we also get the original action of the <leader> key back. Now if you press the leader key it waits for timeout when it has potential mappings, but after timeout finishes it executes its original action. If you don't want to wait, you can now create a different NonRecursive mapping to the leader key. For example if you set your leader to
,, you can do:And this way you get the original action of
,with the\key.The next step with this is to be able to read the mappings from .vimrc that use
<Plug>. I intend to do this by storing all the<Plug>namethat we implement on theRegisterPluginAction, then the vimrc builder can use those stored implemented plugin actions to regex the vimrc for compatible plugin actions. This would mean that we would be able to read the following remaps from vimrc:Which issue(s) this PR fixes
fixes #4916
fixes #2027
fixes #4909
fixes #4973
Special notes for your reviewer:
This PR is based off of the PR #4735. When I tried to base this PR off of the other one it created this PR on my fork. But since I think this PR needs to get attention so people can give their feedback I'm adding it here too.This PR should not be merged before the PR #4735 is merged! And after it gets merged don't try to merge master into this one because I need to first rebase it correctly and then force push again.
I've already rebased this to master, so it is ready to review and merge