Stable Context Menu Shortcuts

.travis.yml

@@ -43,7 +43,6 @@ jobs:
       script:
         # Run the VIM Plugin tests
         - config/travis/vim-plugin-test.sh
-    - php: 7.1
     - php: 7.2
     - stage: docs
       php: 7.2

CHANGELOG.md

@@ -42,6 +42,7 @@ Improvements:
   - [vim-plugin] Better handling of `json_decode` errors
   - [vim-plugin] Add option to switch to open windows
     `g:phpactorUseOpenWindows` - @przepompownia
+  - [vim-plugin] Stable context menu shortcuts - @dantleech (#896)
 
 ## 2019-10-23 0.13.5
 

autoload/phpactor.vim

@@ -713,7 +713,8 @@ function! phpactor#_rpc_dispatch_input(inputs, action, parameters)
     elseif 'choice' == input['type']
         let TypeHandler = function('phpactor#input#choice', [
             \ inputParameters['label'],
-            \ inputParameters['choices']
+            \ inputParameters['choices'],
+            \ inputParameters['keyMap']
         \ ])
     elseif 'list' == input['type']
         let TypeHandler = function('phpactor#input#list', [

autoload/phpactor/input.vim

@@ -20,10 +20,10 @@ function! phpactor#input#confirm(label, ResultHandler)
     call a:ResultHandler(response)
 endfunction
 
-function! phpactor#input#choice(label, choices, ResultHandler)
+let s:usedShortcuts = []
+function! phpactor#input#choice(label, choices, keyMap, ResultHandler)
+    let s:usedShortcuts = []
     let list = []
-    let choices = []
-    let usedShortcuts = []
 
     if empty(a:choices)
         call confirm("No choices available")
@@ -32,22 +32,16 @@ function! phpactor#input#choice(label, choices, ResultHandler)
 
     for choiceLabel in keys(a:choices)
         let buffer = []
-        let foundShortcut = v:false
 
-        for char in split(choiceLabel, '\zs')
-            if v:false == foundShortcut && -1 == index(usedShortcuts, tolower(char))
-                call add(buffer, '&')
-                let foundShortcut = v:true
-                call add(usedShortcuts, tolower(char))
-            endif
-
-            call add(buffer, char)
-        endfor
-
-        let confirmLabel = join(buffer, "")
+        " note that a:keyMap can be an empty list because PHP's json_decode
+        " can't tell the difference between an empty list and an empty dict
+        if !empty(a:keyMap) && has_key(a:keyMap, choiceLabel) && !empty(a:keyMap[choiceLabel])
+            let confirmLabel = s:determineConfirmLabelFromPreference(choiceLabel, a:keyMap[choiceLabel])
+        else
+            let confirmLabel = s:determineConfirmLabel(choiceLabel)
+        endif
 
         call add(list, confirmLabel)
-        call add(choices, choiceLabel)
     endfor
 
     let choice = confirm(a:label, join(list, "\n"))
@@ -57,18 +51,59 @@ function! phpactor#input#choice(label, choices, ResultHandler)
         throw "cancelled"
     endif
 
-    call a:ResultHandler(choices[choice - 1])
+    call a:ResultHandler(keys(a:choices)[choice - 1])
+endfunction
+
+function! s:determineConfirmLabelFromPreference(choiceLabel, preference)
+    let buffer = []
+    let foundShortcut = v:false
+
+    for char in split(a:choiceLabel, '\zs')
+        if foundShortcut == v:false && tolower(char) == tolower(a:preference)
+            let foundShortcut = v:true
+
+            call add(buffer, '&' . a:preference)
+            call add(s:usedShortcuts, a:preference)
+            continue
+        endif
+
+        call add(buffer, char)
+    endfor
+
+    if foundShortcut == v:false
+        " Could not find char in the label - add the shortcut at the end
+        call add(buffer, '&' . a:preference)
+    endif
+
+    return join(buffer, "")
+endfunction
+
+function! s:determineConfirmLabel(choiceLabel)
+    let foundShortcut = v:false
+    let buffer = []
+    for char in split(a:choiceLabel, '\zs')
+        if v:false == foundShortcut && -1 == index(s:usedShortcuts, tolower(char))
+            let foundShortcut = v:true
+
+            call add(buffer, '&')
+            call add(s:usedShortcuts, tolower(char))
+        endif
+
+        call add(buffer, char)
+    endfor
+
+    return join(buffer, "")
 endfunction
 
 function! phpactor#input#list(label, choices, multi, ResultHandler)
     let choices = sort(keys(a:choices))
 
     try
-      let strategy = g:phpactorInputListStrategy
-      call call(strategy, [a:label, choices, a:multi, a:ResultHandler])
+        let strategy = g:phpactorInputListStrategy
+        call call(strategy, [a:label, choices, a:multi, a:ResultHandler])
     catch /E117/
-      redraw!
-      echo 'The strategy "'. strategy .'" is unknown, check the value of "g:phpactorInputListStrategy".'
+        redraw!
+        echo 'The strategy "'. strategy .'" is unknown, check the value of "g:phpactorInputListStrategy".'
     endtry
 endfunction
 

composer.json

@@ -28,7 +28,7 @@
         "phpactor/logging-extension": "~0.2",
         "phpactor/path-finder": "~0.1",
         "phpactor/reference-finder-rpc-extension": "~0.1",
-        "phpactor/rpc-extension": "~0.2",
+        "phpactor/rpc-extension": "^0.2.1",
         "phpactor/source-code-filesystem": "~0.1",
         "phpactor/source-code-filesystem-extension": "~0.1",
         "phpactor/text-document": "~1.1.2",
@@ -39,7 +39,8 @@
         "sebastian/diff": "^3.0",
         "symfony/filesystem": "^3.3",
         "symfony/yaml": "^3.3",
-        "webmozart/glob": "^4.1"
+        "webmozart/glob": "^4.1",
+        "dantleech/invoke": "^1.0.1"
     },
     "require-dev": {
         "symfony/debug": "^4.0",
@@ -66,7 +67,13 @@
     },
     "config": {
         "platform": {
-            "php": "7.1.3"
+            "php": "7.2.0"
         }
+    },
+    "scripts": {
+        "integrate": [
+            "vendor/bin/phpunit",
+            "vendor/bin/php-cs-fixer fix"
+        ]
     }
 }

lib/Extension/CodeTransformExtra/CodeTransformExtraExtension.php

@@ -30,7 +30,6 @@
 use Phpactor\Container\Extension;
 use Phpactor\Extension\CodeTransformExtra\Rpc\ImportMissingClassesHandler;
 use Phpactor\Extension\CodeTransform\CodeTransformExtension;
-use Phpactor\Extension\Core\CoreExtension;
 use Phpactor\Extension\Logger\LoggingExtension;
 use Phpactor\Extension\Php\Model\PhpVersionResolver;
 use Phpactor\Extension\Rpc\RpcExtension;

lib/Extension/ContextMenu/ContextMenuExtension.php

@@ -7,6 +7,7 @@
 use Phpactor\Container\Extension;
 use Phpactor\Container\ContainerBuilder;
 use Phpactor\Container\Container;
+use Phpactor\Extension\ContextMenu\Model\ContextMenu;
 use Phpactor\Extension\Rpc\RpcExtension;
 use Phpactor\Extension\WorseReflection\WorseReflectionExtension;
 use Phpactor\MapResolver\Resolver;
@@ -25,7 +26,7 @@ public function load(ContainerBuilder $container)
                 $container->get(WorseReflectionExtension::SERVICE_REFLECTOR),
                 $container->get(CodeTransformExtension::SERVICE_CLASS_INTERESTING_OFFSET_FINDER),
                 $container->get('application.helper.class_file_normalizer'),
-                json_decode(file_get_contents(__DIR__ . '/menu.json'), true),
+                ContextMenu::fromArray(json_decode(file_get_contents(__DIR__ . '/menu.json'), true)),
                 $container
             );
         }, [ RpcExtension::TAG_RPC_HANDLER => ['name' => ContextMenuHandler::NAME] ]);

lib/Extension/ContextMenu/Handler/ContextMenuHandler.php

@@ -3,6 +3,8 @@
 namespace Phpactor\Extension\ContextMenu\Handler;
 
 use Phpactor\CodeTransform\Domain\Helper\InterestingOffsetFinder;
+use Phpactor\Extension\ContextMenu\Model\Action;
+use Phpactor\Extension\ContextMenu\Model\ContextMenu;
 use Phpactor\MapResolver\Resolver;
 use Phpactor\Extension\Rpc\Handler;
 use Phpactor\TextDocument\ByteOffset;
@@ -38,7 +40,7 @@ class ContextMenuHandler implements Handler
     private $offsetFinder;
 
     /**
-     * @var array
+     * @var ContextMenu
      */
     private $menu;
 
@@ -56,7 +58,7 @@ public function __construct(
         Reflector $reflector,
         InterestingOffsetFinder $offsetFinder,
         ClassFileNormalizer $classFileNormalizer,
-        array $menu,
+        ContextMenu $menu,
         Container $container
     ) {
         $this->reflector = $reflector;
@@ -98,14 +100,14 @@ public function handle(array $arguments)
 
     private function resolveAction(ReflectionOffset $offset, Symbol $symbol, array $arguments)
     {
-        if (false === isset($this->menu[$symbol->symbolType()])) {
+        if (false === $this->menu->hasContext($symbol->symbolType())) {
             return EchoResponse::fromMessage(sprintf(
                 'No context actions available for symbol type "%s"',
                 $symbol->symbolType()
             ));
         }
 
-        $symbolMenu = $this->menu[$symbol->symbolType()];
+        $symbolMenu = $this->menu->forContext($symbol->symbolType());
 
         if (null !== $arguments[self::PARAMETER_ACTION]) {
             return $this->delegateAction($symbolMenu, $arguments, $offset);
@@ -121,8 +123,8 @@ private function delegateAction(array $symbolMenu, array $arguments, ReflectionO
         // to avoid a cyclic dependency we get the request handler from the container ...
         return $this->container->get(ContextMenuExtension::SERVICE_REQUEST_HANDLER)->handle(
             Request::fromNameAndParameters(
-                $action[self::PARAMETER_ACTION],
-                $this->replaceTokens($action['parameters'], $offset, $arguments)
+                $action->action(),
+                $this->replaceTokens($action->parameters(), $offset, $arguments)
             )
         );
     }
@@ -143,7 +145,9 @@ private function actionSelectionAction(Symbol $symbol, $symbolMenu, array $argum
                     self::PARAMETER_ACTION,
                     sprintf('%s "%s":', ucfirst($symbol->symbolType()), $symbol->name()),
                     array_combine(array_keys($symbolMenu), array_keys($symbolMenu))
-                )
+                )->withKeys(array_combine(array_keys($symbolMenu), array_map(function (Action $action) {
+                    return $action->key();
+                }, $symbolMenu)))
             ]
         );
     }

lib/Extension/ContextMenu/Model/Action.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace Phpactor\Extension\ContextMenu\Model;
+
+class Action
+{
+    /**
+     * @var string
+     */
+    private $action;
+    /**
+     * @var string|null
+     */
+    private $key;
+    /**
+     * @var array
+     */
+    private $parameters;
+
+    public function __construct(string $action, ?string $key = null, array $parameters = [])
+    {
+        $this->action = $action;
+        $this->key = $key;
+        $this->parameters = $parameters;
+    }
+
+    public function action(): string
+    {
+        return $this->action;
+    }
+
+    public function parameters(): array
+    {
+        return $this->parameters;
+    }
+
+    public function key(): ?string
+    {
+        return $this->key;
+    }
+}