company-auto-update-doc leaves open the buffer that it used

[vemv]

Hi,

TIL about company-auto-update-doc. Very handy!

However, after I'm done completing (whether it is by successfully choosing a completion, or aborting my intent), the buffer that was created, in my case *cider-doc*, remains open. This is problematic because leaving a buffer that wasn't there before displaces my other buffers.

This isn't the case if using <f1> (company-show-doc-buffer) manually with each completion - with f1, the *cider-doc* buffer always ends up closed.

I can show a GIF if that helps, but I believe the description is clear enough.

Finally, in technical terms, CIDER is specifying :company-doc-buffer #'cider-create-doc-buffer. We (the CIDER maintainers) certainly are completely fine with this ancilliary buffer being destroyed. It's very cheap to re-create.

Thanks - V

[dgutov]

@nemethf What do you think about changing/fixing the behavior to close the doc buffer at the end even when company-auto-update-doc is non-nil?

[dgutov]

@vemv IIUC the issue is not whether the ancillary buffer is destroyed (it won't be), but whether it remains displayed in the window.

[nemethf]

@nemethf What do you think about changing/fixing the behavior to close the doc buffer at the end even when company-auto-update-doc is non-nil?

The documentation is helpful to find the right item to select. But often I also consult the documentation after an item has been selected. It helps me to fill out the arguments of a function call, for example. When I'm finished I usually just press C-x 1.

However, I understand how the current behavior can annoy others.

Maybe a defcustom named something like company-keep-doc-buffer-visible would make everybody happy. Except you, the maintainer, as it would introduce complexity to the code.

[dgutov]

I see.

Well, it doesn't have to be a separate var: the current one could grow the value persistent, or something like that.

If you do find the current behavior good, I suppose we will live with an extra condition.

[nemethf]

So C-u <f1> would cycle through three options instead of the current behavior when it toggles between on and off?

I can try to prepare a pull request. (But I cannot promise I will be fast.)

[dgutov]

Hmm, good question. What if we used a different value of prefix, such as C-u C-u, for one of the values?

[vemv]

FWIW, I figured that CIDER (the library, not my personal .emacs.d) could perform:

(add-hook 'company-after-completion-hook (lambda (_)
                                           (when-let ((b (get-buffer "*cider-doc*")))
                                             (bury-buffer b))))

which I've verified to work.

It seems cheap enough (with the caveat that we don't take for granted that Compliment exists - it's an optional library).

However it's not exactly clean, as it might bury a previously existing buffer.

I do genuinely believe that a good chunk of users would be surprised by the current default behavior, so a generalized fix/option might be well worthwhile.

[dgutov]

I do genuinely believe that a good chunk of users would be surprised by the current default behavior, so a generalized fix/option might be well worthwhile.

That's also what I was thinking.

[nemethf]

In the end, I took a different approach. In the pull request, auto-update-doc always restores the window configuration when company finishes. But I also added a new command that shows the documentation of the selected item after exiting. This seemed more natural than the C-u C-u <f1> approach. What do you think?

[vemv]

FWIW, it looks reasonable to me. It's the first time I hear of set-window-configuration, however I see it's already in use in Company, so I guess it's a trusted mechanism by now

[dgutov]

I like the suggestion, but I'd prefer a shorter patch. And company--saved-window-configuration seems like it's duplicating the purpose of company--electric-saved-window-configuration. Does the patch below work for you as well (including performance)?

Regarding the key-binding, though. I'm of the school of thought that believes that M-RET, same as Alt-Enter, should (un)maximize the window. Do you have other suggestions, maybe? I was thinking you'd just replace the binding, but maybe using it strategically also has a point. We could add a prefix argument to company-complete, but that's both slower and would preclude us from doing something else with it (such as inhibiting template expansion, for example).

diff --git a/company.el b/company.el
index 212ff13..c0e6bb2 100644
--- a/company.el
+++ b/company.el
@@ -1360,9 +1360,6 @@ To toggle the value of this variable, call `company-show-doc-buffer' with a
 prefix argument.")
 
 (defun company-call-frontends (command)
-  (when (and company-auto-update-doc
-             (memq command '(update show)))
-    (company-show-doc-buffer))
   (cl-loop for frontend in company-frontends collect
            (condition-case-unless-debug err
                (funcall frontend command)
@@ -2227,7 +2224,10 @@ For more details see `company-insertion-on-trigger' and
             (let (company-idle-delay) ; Against misbehavior while debugging.
               (company--perform)))
           (if company-candidates
-              (company-call-frontends 'post-command)
+              (progn
+                (company-call-frontends 'post-command)
+                (when company-auto-update-doc
+                  (company-show-doc-buffer)))
             (let ((delay (company--idle-delay)))
              (and (numberp delay)
                   (not defining-kbd-macro)
@@ -2641,6 +2641,13 @@ For use in the `select-mouse' frontend action.  `let'-bound.")
     (let ((result (nth company-selection company-candidates)))
       (company-finish result))))
 
+(defun company-complete-selection-show-doc ()
+  "Insert the selected candidate and show its documentation."
+  (interactive)
+  (company--show-doc-buffer)
+  (company-complete-selection)
+  (setq company--electric-saved-window-configuration nil))
+
 (defun company-complete-common ()
   "Insert the common part of all candidates."
   (interactive)
@@ -2883,10 +2890,8 @@ automatically show the documentation buffer for each selection."
   (interactive "P")
   (when toggle-auto-update
     (setq company-auto-update-doc (not company-auto-update-doc)))
-  (if company-auto-update-doc
-      (company--show-doc-buffer)
-    (company--electric-do
-      (company--show-doc-buffer))))
+  (company--electric-do
+    (company--show-doc-buffer)))
 (put 'company-show-doc-buffer 'company-keep t)
 
 (defun company-show-location ()

[nemethf]

Regarding the key-binding, though. I'm of the school of thought that believes that M-RET, same as Alt-Enter, should (un)maximize the window. Do you have other suggestions, maybe?

There are examples in the Emacs codebase of M-RET. For example, both message-newline-and-reformat or (a bit more relevant here) minibuffer-choose-completion are bound to M-RET.

On the other hand, what do you think about binding company-complete-selection-show-doc to C-j. That seems logical and easy to press even in a tty.

[dgutov]

There are examples in the Emacs codebase of M-RET. For example, both message-newline-and-reformat or (a bit more relevant here) minibuffer-choose-completion are bound to M-RET.

Not much I can do about those. Luckily, the contexts they work in are rare enough.

On the other hand, what do you think about binding company-complete-selection-show-doc to C-j. That seems logical and easy to press even in a tty.

I like the general direction, but maybe not the exact choice. In Emacs's minibuffer completion, C-j either means "insert current input verbatim" or "choose the current completion". What about M-j or C-M-j? In ivy-minibuffer-map, the former has something to do with text (ivy-yank-word), but there is no counterpart for in-buffer completion. The downside is that we'd be taking the text editing binding (default-indent-new-line), but I suppose a conflict would be pretty rare.

In any case, we don't have to choose the binding right now. Adding the command unbound is also a good option.

[vemv]

As far as my testing went, this issue is now solved.

[dgutov]

I think we'll keep this one open for now, to finish the discussion of adding the new command (or a different optional behavior).

[nemethf]

I like the general direction, but maybe not the exact choice. In Emacs's minibuffer completion, C-j either means "insert current input verbatim" or "choose the current completion". What about M-j or C-M-j? In ivy-minibuffer-map, the former has something to do with text (ivy-yank-word), but there is no counterpart for in-buffer completion. The downside is that we'd be taking the text editing binding (default-indent-new-line), but I suppose a conflict would be pretty rare.

I cannot explain it, but I find M-RET easy to type after C-n/C-p and M-j hard to type. Would it be OK to bind M-RET as well? if xterm shadows it, users could use M-j.

What do you think about C-c RET?

Also, we try to find a binding for company-complete-selection-show-doc, but maybe a company-complete-selection-show-location would be useful as well. Or simply a company-complete-selection-keep-windows.

This is a tangent, but I wanted to check the current keybindings of company and the usual C-h b did not work since C-h is remapped to company-show-doc-buffer. If company gains more keybindings, it would be good to be able to quickly get a reminder of its keybindings.

In any case, we don't have to choose the binding right now. Adding the command unbound is also a good option.

That sounds good as well. Thanks.

[dgutov]

I cannot explain it, but I find M-RET easy to type after C-n/C-p and M-j hard to type. Would it be OK to bind M-RET as well? if xterm shadows it, users could use M-j.

My own bias is that M-RET is used by MS Windows (which has some users), and I have a personal binding in the global map that mirrors it (as a memento, I suppose). Any minor mode using it would shadow the global one, for any users that set it up just like me.

But if some others also really like it, and there is some semantic precedent (minibuffer-choose-completion is somewhat relevant, but there is nothing doc-related in it), I can be persuaded.

What do you think about C-c RET?

https://emacsdocs.org/docs/emacs/Key-Bindings says C-c followed by character are reserved, though I'm not sure if RET is considered a character. I think it's an option if you actually like it.

There are two other actions (currently not implemented yet) that we should keep in mind when picking bindings:

• Insert with replacing prefix (or without, if replacing will be the default). lsp-mode uses C-u RET ([3.16] Insert replace support emacs-lsp/lsp-mode#2711).
• Insert without expanding the snippet (e.g. inserting just the function name, without the arguments).

Also, we try to find a binding for company-complete-selection-show-doc, but maybe a company-complete-selection-show-location would be useful as well. Or simply a company-complete-selection-keep-windows.

Right, I was also wondering whether company-show-location needs a corresponding var. But it doesn't seem as popular, so maybe not. company-complete-selection-keep-windows can keep both kinds of windows indeed.

Also see #310 (I just noticed it yesterday when going through old reports).

This is a tangent, but I wanted to check the current keybindings of company and the usual C-h b did not work since C-h is remapped to company-show-doc-buffer. If company gains more keybindings, it would be good to be able to quickly get a reminder of its keybindings.

Both good points. Any suggestions?

The binding was changed from <f1> in #129 10 years ago, and so far this only came up once (#454). So while there is a good technical argument for changing it, I'm hesitant to do it without a good, easy-to-press alternative. Preferably, somehow consistently fitting in with the rest of the bindings.

lsp-mode uses s-l h h, auto-complete uses/used C-?, eglot actively avoids choosing bindings.

I suppose if we implement #185 with a short binding like C-c C-d (making it easier to remember), that would make it a candidate when the popup is active, too. Alternatively, implementing a hovering documentation popup beside the completions list as a built-in option would make the command to toggle it less important, too (then it could have a longer binding).

[dgutov]

I just got a handy reminder than company-quickhelp uses M-h as a binding to show the popup manually. Looks like a decent option, too.

[vemv]

(I'll get back to this asap)