emacs-lsp / lsp-ui

UI integrations for lsp-mode
https://emacs-lsp.github.io/lsp-ui
GNU General Public License v3.0
1.04k stars 139 forks source link

Conflicts with helm and solarized-theme #33

Open twlz0ne opened 6 years ago

twlz0ne commented 6 years ago

Expected Behavior

helm CAN accept user input or keypresses

Actual Behavior

helm CANNOT accept user input or keypresses

Steps to reproduce the behavior

  1. Download gist: test-lsp-ui.el
  2. /path/to/emacs -Q -l /path/to/test-lsp-ui.el, wait for the helm window to appear
  3. Press any key

Environment

twlz0ne commented 6 years ago

There are two solutions, but I do not understand its underlying causes.

  1. Override the function lsp-ui-peek--make-footer in lsp-ui/lsp-ui-peek.el#L204:

        ;; delete line below or change `:height' to a number other than 0.5 to fix issue.
    -     (propertize "\n" 'face '(:height 0.5))

    Visit here to check out the context of modification: https://github.com/twlz0ne/lsp-ui/commit/2b86af41e25b7b3d2d1ef7f95994ccba6e1197d5.

  2. Or change the difinition of mode-line in solarized-theme/solarized.el#L298:

        `(mode-line
        ((,class (:inverse-video unspecified
                                    :overline ,s-mode-line-bg
                                    :underline ,s-mode-line-underline
                                    :foreground ,s-mode-line-fg
                                    :background ,s-mode-line-bg
                                    ;; delete lines below or change the `:line-width' to \
                                    ;; a number other than 1 and 0 to fix issue.
    -                               :box (:line-width 1
    -                                                 :color ,s-mode-line-bg
    -                                                 :style unspecified)
                                    ))))
sebastiencs commented 6 years ago

Wow that's a weird bug. Thanks for the detailed report ! I will look into it

sebastiencs commented 6 years ago
            (run-with-idle-timer 2 nil 'keypress-down)
            (run-with-idle-timer 3 nil 'keypress-up)
-           (run-with-idle-timer 4 nil 'keypress-abort)
+           (run-with-idle-timer 4 nil 'lsp-ui-peek--abort)
            (run-with-idle-timer 5 nil 'helm-mini)
            ))

I found out that if you change this in your gist, the bug disappears. So it's something related to the keymap (lsp-ui-peek-mode-map).

It seems that disabling lsp-ui-peek-mode with a command in the keymap makes something wrong, but I have no idea why (+ the fact that it needs solarized-theme).

sebastiencs commented 6 years ago

The issue should be fixed with https://github.com/emacs-lsp/lsp-ui/commit/a30139eab6fb19173b9e1fe5ef3ae8b75de5a5a5 Can you confirm ?

jiegec commented 6 years ago

This seems really weird..

twlz0ne commented 6 years ago

@sebastiencs

The issue should be fixed with a30139e Can you confirm ?

The bug appears when I invoke helm again.

twlz0ne commented 6 years ago

@sebastiencs It looks like the lsp-ui-doc frame intercepted keyboard events, causing helm not working.

I simplified the testing script, make the problem more obvious:

  1. Re download gist: test-lsp-ui.el
  2. Delete ~/.emacs.d/elpa--test-lsp if it exists
  3. /path/to/emacs -Q -l /path/to/test-lsp-ui.el, and do the following in order:

    ;; 1
    (lsp-ui-doc--display 'foo \"bar\")
    
    ;; 2
    (call-interactively 'helm-M-x)
    
    ;; 3
    ;; Press `C-n' / `C-p'
    ;; `helm' CANNOT accept user input or keypresses
    
    ;; 4
    (lsp-ui-doc--delete-frame)
    
    ;; 5
    (call-interactively 'helm-M-x)
    
    ;; 6
    ;; Press `C-n' / `C-p'
    ;; `helm' CAN accept user input or keypresses

Perhaps we should invite @thierryvolpiatto to participate in the discussion.

sebastiencs commented 6 years ago

I don't have any issue.. Can you try on a linux system ? It could be a bug with the mac os implementation.

Also the child frame shouldn't intercept any events: https://github.com/emacs-lsp/lsp-ui/blob/b40931618e3b19fdfbcfa3e83ddba97d04f334af/lsp-ui-doc.el#L456

MaskRay commented 6 years ago

My helm-selection-overlay does not move in the helm buffer . This issue bothered me much and made me switch to Ivy 😂 (@twlz0ne said it was because `helm' CANNOT accept user input or keypresses)

twlz0ne commented 6 years ago

@sebastiencs Linux indeed does not have this issue.

I also tested Windows, the result is the same as macOS.

Since this is an issue related to os implementation, we can only wait for Emacs update, until then, it can be circumvented by the following configuration:

(add-hook 'helm-major-mode-hook 'lsp-ui-doc--delete-frame)
sebastiencs commented 6 years ago

@twlz0ne You should probably report the bug to bug-gnu-emacs@gnu.org

arejensen commented 6 years ago

@sebastiencs @twlz0ne I'm able to replicate the issue on Linux with Emacs 26.0.50 and Emacs 27.0.50.

In addition I noticed a marked slowdown with in-buffer navigation after the doc child-frame was initially displayed. (Independent of theme.) This was true when using avy or helm so it does not seem to be a helm related issue. (Initially I suspected this to be an issue with my tiling window manager (exwm, stumpwm), but the problem persists in xfce4.)

I traced both problems down to the make-frame-invisible call in lsp-ui-doc--hide-frame. Replacing it with a call to lsp-ui-doc--delete-frame makes the solarized theme work without the hook above, while also making the slowdowns go away.

I've created a pull request with the fix (#88) , but I don't recommend to accept it as-is since recreating the frame every time leads to its own set of problems.

I realize that this can be split into a comment on this bug report (I can replicate the issue on Linux) and its own bug report (slowdowns in navigation). Let me know if you want me to do so.

thierryvolpiatto commented 6 years ago

Are Jensen notifications@github.com writes:

@sebastiencs @twlz0ne I'm able to replicate the issue on Linux with Emacs 26.0.50 and Emacs 27.0.50.

I traced both problems down to the make-frame-invisible call in lsp-ui-doc--hide-frame. Replacing it with a call to lsp-ui-doc--delete-frame makes the solarized theme work without the hook above, while also making the slowdowns go away.

I don't know exactly what you are doing but I have noticed a regression with emacs-26/27 around make-frame and/or raise-frame functions that are very slow compared to emacs-25. HTH.

-- Thierry

sebastiencs commented 6 years ago

@arejensen Thanks for looking into this. However I don't think that deleting the frame is the right solution, it makes recreate the frame every time and it's very slow..

I will revert https://github.com/emacs-lsp/lsp-ui/commit/3f0934633b1bba8d02a4f71402886c5a72e78b54 and ask for help on bug-gnu-emacs@gnu.org.

MaskRay commented 6 years ago

The yellow remnant of the evil-mode block cursor in the top left appears when the child frame is created but disappears afterwards. 3f09346 makes the cursor appear every time a new child frame is created...

hyiltiz commented 3 years ago

This issue also exists with ivy with the following version:

     Status: Installed in ‘lsp-ui-20201024.2307/’ (unsigned). Delete
    Version: 20201024.2307
     Commit: b1693d610c4d2c44305eba2719e8d4097fdcdcb8
    Summary: UI modules for lsp-mode
   Requires: emacs-26.1, dash-2.14, dash-functional-1.2.0, lsp-mode-6.0, markdown-mode-2.3
thierryvolpiatto commented 3 years ago

Hörmet Yiltiz notifications@github.com writes:

This issue also exists with ivy.

See https://github.com/emacs-helm/helm/issues/1976

-- Thierry