[lsp] Updated keybindings w.r.t upstream lsp-mode refactoring / additions.

[cormacc]

lsp-mode had renamed lsp-shutdown-workspace and lsp-restart-workspace.

Added bindings for the following newer interactive lsp-mode functions:

• lsp-organize-imports
• lsp-document-highlight
• lsp-lens-show
• lsp-lens-hide

Declared a new major mode prefix (x: text/code).
Moved execute-code-action binding under new prefix

Tidy-up in preparation for a c-c++ layer update (adding clangd support)

[yyoncho]

Looks good to me!

Would you add shortcut also for lsp-treemacs-symbols?

I personally would pick another shortcut for executing code actions ,xa e. g. ,ea.

[cormacc]

Bound lsp-treemacs-symbols under ,GS
Re. ,ea -- that was my own first thought, but occurred to me that ,e is used for eval in various lisp (?and other?) layers, so ,x maybe safer. And a code action is an action on the text document, so to speak...

While you're here @yyoncho -- just noticed that the helm-lsp-*-workspace-symbol bindings are broken -- appears to be a package lazy-loading problem, but think it used to work -- any insights?

Relevant section of packages.el is simply....

(defun lsp/init-helm-lsp ()
  (use-package helm-lsp :defer t))

[yyoncho]

While you're here @yyoncho -- just noticed that the helm-lsp-*-workspace-symbol bindings are broken -- appears to be a package lazy-loading problem, but think it used to work -- any insights?

I am unable to reproduce following the instructions from https://github.com/emacs-lsp/lsp-docker#spacemacs (it runs clean spacemacs in a docker image with emacs 27.0.5 with several preinstalled language servers).

[cormacc]

I'm an idiot -- had the Ivy layer enabled :)
On the plus side, that tells us those keybindings should always have been conditional. Amended/pushed -- thanks @yyoncho

[yyoncho]

One more thing to consider about the execute code actions - at some point it will be split into 3 methods - execute refactoring(e.g. extract local)/execute source action (e. g. rename file)/execute quick fix(e. g. add missing import).

[smile13241324]

Given the future actions execute would split into I would expect these under r for refactoring. Especially given these standard IDE actions extract local, rename, quick fix.

Can we move executing code actions to ,ra instead? I think this would be more conventional then.

Refactoring
Refactoring prefix is SPC m r.

[yyoncho]

Can we move executing code actions to ,ra instead?

That would collide with java layer:

        "rai" 'lsp-java-add-import
        "ram" 'lsp-java-add-unimplemented-methods
        "rat" 'lsp-java-add-throws
        "raa" 'lsp-java-assign-all
        "raf" 'lsp-java-assign-to-field

We may use
,af - Action Fix
,as - Action Source
,ar - Action Refactor.
,aa - All Actions (current method).

My personal preference is to have execute action to something more ergonomic(I find ,xa more difficult to press) since I use this very often.

[smile13241324]

Good point.
Lets use the suggested ,a bindings then. I find them also more egonomic then ,xa.

[cormacc]

Fine by me -- away from my dev machine, but will move ,xa to ,aa later this eve / tomorrow morning.

Should I also add a note to the README indicating that ,af, ,as, ,ar are reserved for lsp-mode additions, to forestall other layers squatting on them?

[smile13241324]

@cormacc that would be great. By the way while you are at it, would you mind adding a line to CHANGELOG.develop as well?

[cormacc]

Moved the execute code action binding and added placeholders for fix, refactor and source.

Added a changelog entry -- realised I'd made no entry in relation to my original lsp layer PR some time ago so just created a single entry related to that -- seems redundant to create two entries given there's been no release since, though happy to elaborate if you want?

[cormacc]

@sdwolfz The circleci failures are opaque to me -- does the reference to stable elpa mean I'm using packages that aren't on stable elpa or something? Or that my changes are incompatible with the package versions on stable elpa?

[sdwolfz]

(defun configuration-layer//stable-elpa-verify-archive ()
  "Verify the downloaded stable ELPA repository archive.

Returns non nil if the verification succeeded.

If Spacemacs cannot verify the archive a prompt ask the user if they want to
continue with the stable ELPA repository installation."

Looks like there might be something wrong with the downloaded archive... nothing under your control. @syl20bnr has done some changes in that area recently.

[smile13241324]

Looks good, I'll make a quick check, if nothing goes wrong it will be merged in no time.

[smile13241324]

Thank you for contributing to Spacemacs 💜, I have cherry picked your commit to develop. You can safely remove your branch.

[yyoncho]

@cormacc

had the Ivy layer enabled :)
On the plus side, that tells us those keybindings should always have been conditional.

We now have an initial version of ivy support at https://github.com/emacs-lsp/lsp-ivy in case you are interested.

[cormacc]

Sounds good @yyoncho -- I'm usually using Ivy as helm-projectile-find-file chokes on the main (C) codebase I work on. I'll take a look and probably raise another PR shortly. I've one against the c/c++ layer I'd like to conclude first though -- branch switching for parallel PRs gives me a headache :)

[NANASHI0X74]

We may use
,af - Action Fix
,as - Action Source
,ar - Action Refactor.
,aa - All Actions (current method).

I noticed these are placeholders. Are there any plans to implement these different kinds of code actions in lsp-mode? Is there some kind of roadmap where they're described? I couldn't find one

A month ago, the documentation was updated to say that these are reserved bindings for 'imminent lsp-mode features' (See mention above).

[smile13241324]

@NANASHI0X74 these have originally be introduced by yyoncho to support latest lsp-mode features. I am pretty sure that they are finished by now but nobody updated the bindings yet, feel free to open a PR for this.

The task includes looking up the proper function in lsp-mode and replacing the place holder bindings in lsp-layer.