search

company-mode / company-quickhelp

Documentation popup for Company
GNU General Public License v3.0
374 stars 33 forks source link

popup mishandled by display-buffer-alist #126

Closed rdiaz02 closed 7 months ago

rdiaz02 commented 1 year ago

This is not a bug, but just an unfortunate interaction with mechanisms for selecting where to display buffers.

My display-buffer-alist has this entry

 ("\\*Help\\*" (display-buffer-reuse-mode-window
                      ace-display-buffer))

where ace-display-buffer is a function from ace-window (https://github.com/abo-abo/ace-window) that shows a character in each window, so one can select where to display a buffer.

If I have no visible window displaying help, when the quickhelp is to be displayed, ace-display-buffer is called, it places numbers on all windows, and waits for input (a number) to select a window where the help should be displayed (and then I need to C-g to, I guess, interrupt the ace-display-buffer process and the popup gets displayed). This is an image of what happens (the red numbers on each window are the numbers produced by ace-display-buffer).

cqh

I would have expected the popup to be displayed directly.

In addition to removing the entry for help buffers in display-buffer-alist, is there anything I can do to prevent this?

EDIT: looking at the code, in line 142 and ff. of company-quickhelp.el, it says " The company backend can either return a buffer with the doc or a cons containing the doc buffer and a position at which to start reading".

So I guess my problem is not due to an interaction between my display-buffer-alist and company-quickhelp, but between my display-buffer-alist and the company backends. In fact, I experience this problem with some languages (e.g., elisp) but not others (e.g., R).

Anyway, I would appreciate any suggestions (and, in case it matters, I experience this same issue with company-posframe, https://github.com/tumashu/company-posframe, but I do not experience this issue with company-box, https://github.com/sebastiencs/company-box, but then the mechanism could be very different).

dgutov commented 11 months ago

Perhaps ace-window's author will have some recommendations?

rdiaz02 commented 10 months ago

I tried, but I was not able to open a new issue.

Anyway, I am not sure I understand what happens and why, so I am leaving here code to reproduce the issue and hopefully a better explanation. But I am closing the issue for now, at least until I understand it better. The problem is of course solved not having ace-display-buffer display Help buffers, which might actually be a more reasonable thing to do.

So I do not understand why the quickhelp seems to be calling ace-display-buffer, even when the quichelp is not displayed as a regular buffer. Nor do I understand why display-buffer-at-bottom prevents calling ace-display-buffer, but that does not happen with display-buffer-pop-up-window

Interestingly, ace-display-buffer is not called if I use the company-box package (https://github.com/sebastiencs/company-box) instead of company-quickhelp.

Code to reproduce the issue 

(require 'package)
(package-initialize)

(use-package company
  :ensure t
  :init (global-company-mode 1)
  :config
  (progn
    (company-prescient-mode 1)
    (setq company-show-quick-access t)
    (setq company-minimum-prefix-length 2)
    (setq company-idle-delay .5) 
    (setq company-require-match nil)
    (setq company-selection-wrap-around 't)
    (setq company-tooltip-limit 10) 
    (setq company-dabbrev-downcase nil)
    (setq company-auto-update-doc nil)
    ))

(use-package company-quickhelp
  :ensure t
  :after company
  :init (company-quickhelp-mode 1)
  :config
  (progn
    (setq company-quickhelp-delay 1)
  ))

(use-package ace-window
  :ensure t
  :commands (ace-window
         ace-display-buffer
         )
  :init
  (progn
    (setq aw-dispatch-always t)
    )
  :config
  (progn
    (setq aw-minibuffer-flag nil)
    (setq aw-background nil)
    (set-face-attribute 'aw-leading-char-face nil
            :height 8.5 :weight 'bold)
    ))

(add-to-list 'display-buffer-alist
         '("\\*Help\\*"
           (display-buffer-reuse-window
                display-buffer-pop-up-window
        ;; display-buffer-at-bottom ;; this prevents ace-display issue
        ace-display-buffer
        )))
dgutov commented 10 months ago

Interestingly, ace-display-buffer is not called if I use the company-box package (https://github.com/sebastiencs/company-box) instead of company-quickhelp.

Hmm, perhaps following its example here would be a good idea. Could you try this patch?

diff --git a/company-quickhelp.el b/company-quickhelp.el
index 17b32a9..dd21bc5 100644
--- a/company-quickhelp.el
+++ b/company-quickhelp.el
@@ -130,7 +130,8 @@ just grab the first candidate and press forward."

 (defun company-quickhelp--fetch-docstring (selected)
   "Fetch docstring from the current backend for SELECTED string."
-  (let ((quickhelp-str (company-call-backend 'quickhelp-string selected)))
+  (let* ((display-buffer-alist nil)
+         (quickhelp-str (company-call-backend 'quickhelp-string selected)))
     (if (stringp quickhelp-str)
         (with-temp-buffer
           (insert quickhelp-str)

Also, does C-h pressed when the company popup is active exhibit the same problem? Perhaps we should add a similar like to company--show-doc-buffer too.

rdiaz02 commented 10 months ago

Thanks! That solves the problem of calling ace-display-buffer when the quickhelp is displayed, initially. However, if one keeps using it, opens Help buffers and, calls C-u C-h, then it starts misbehaving again and remains broken in subsequent invocations (before we call C-u C-h again, but that is because there is already a Help buffer, even if not displayed among the visible windows).

Pressing C-h still calls ace-display-buffer , but then that is what should happen. (The behavior is somewhat jarring, and one needs to press a window number twice, not once).

The more I think about this, the more I think this really isn't worth digging into: probably the quickhelp should be left alone by ace. But then, handling Help buffers with ace and expecting it to also work smoothly with C-h and C-u C-h cannot be satisfactory. In particular C-u C-h: we are asking to keep the help updated, so it necessarily has to update a Help buffer, and we have already said that should go through ace, and it does.

I really think this is not solvable in a satisfactory way, and it is not worth more time. I really apologize for the noise.

dgutov commented 10 months ago

That's too bad.

Pressing C-h still calls ace-display-buffer , but then that is what should happen.

If you're sure about that, I suppose there's not much to be done.

OTOH, you could try this additional patch:

diff --git a/company.el b/company.el
index 7060b60..6d8f9e9 100644
--- a/company.el
+++ b/company.el
@@ -3129,6 +3129,7 @@ from the candidates list.")
 (defun company--show-doc-buffer ()
   "Show the documentation buffer for the selection."
   (let ((other-window-scroll-buffer)
+        (display-buffer-alist nil)
         (selection (or company-selection 0)))
       (let* ((selected (nth selection company-candidates))
              (doc-buffer (or (company-call-backend 'doc-buffer selected)

This change could also be predicated on whether company-auto-update-doc is non-nil.

rdiaz02 commented 10 months ago

Pressing C-h still calls ace-display-buffer , but then that is what should happen.

If you're sure about that, I suppose there's not much to be done.

I think it cannot (and should not) be otherwise. C-h is company-show-doc-buffer which shows the documentation buffer for the selection. And if in display-buffer-alist the first entry we have for help were ace-display-buffer then it must ask us for where to show the buffer.

The change in https://github.com/company-mode/company-quickhelp/issues/126#issuecomment-1826178909 is useful to prevent quickhelp itself from triggering ace-display-buffer.

But I think the company mechanism for displaying the help should follow the display-buffer-alist setting.

OTOH, you could try this additional patch:

diff --git a/company.el b/company.el
index 7060b60..6d8f9e9 100644
--- a/company.el
+++ b/company.el
@@ -3129,6 +3129,7 @@ from the candidates list.")
 (defun company--show-doc-buffer ()
   "Show the documentation buffer for the selection."
   (let ((other-window-scroll-buffer)
+        (display-buffer-alist nil)
         (selection (or company-selection 0)))
       (let* ((selected (nth selection company-candidates))
              (doc-buffer (or (company-call-backend 'doc-buffer selected)

This change could also be predicated on whether company-auto-update-doc is non-nil.

But wouldn't this make the consequences of different values of company-auto-update-doc harder to understand?

dgutov commented 10 months ago

But wouldn't this make the consequences of different values of company-auto-update-doc harder to understand?

That might indeed be a problem, if some users have display-buffer-alist entries that work automatically without user's intervention and thus are compatible with company-auto-update-doc=t.

Perhaps a solution is for you to customize display-buffer-alist in such a way that checks that company-candidates is non-nil (which would mean that completion is active), and when so, skips the application of ace-display-buffer. But, perhaps, excepting the case when this-command is eq to company-show-doc-buffer.

rdiaz02 commented 7 months ago

Sorry for the long delay: I missed this. I think we probably should close the issue, as this is really a contorted corner case (and I've since stopped having ace control help buffers, and instead always show them in the bottom, which is a lot simpler and more reliable). The solution, if any, I think is the last one you suggested: customize display-buffer-alist, and this is outside the scope of company-quickhelp. I think we might want to close the issue.

dgutov commented 7 months ago

Sure, no problem. We can revisit this if somebody else reports the same difficulty.