Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add global entry #626

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add global entry #626

wants to merge 3 commits into from

Conversation

jcs090218
Copy link
Member

For #625.

Not sure if this is a good idea, but I have implemented first.

Copy link
Member

@ericdallo ericdallo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I don't know if there is any side effects with that, WDYT @yyoncho?

Copy link
Member

@yyoncho yyoncho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need something like this?

(defun lsp-ui-doc-disable (args)
  "..."
  (interactive "P")
  (setq lsp-ui-doc-enable nil)
  (mapc (lambda (b) (with-current-buffer b
                      (lsp-ui-doc-mode -1)))
        (buffer-list)))

for the enable we have to look for all buffers in lsp-mode state.

@jcs090218
Copy link
Member Author

for the enable we have to look for all buffers in lsp-mode state.

No, I think the global minor mode will handles this? But the flag would not disable since it's defcustom.

(define-global-minor-mode global-lsp-ui-doc-mode lsp-ui-doc-mode
  (lambda () (lsp-ui-doc-mode 1)))

@yyoncho
Copy link
Member

yyoncho commented Jun 18, 2021

Ok, seems like the global mode will handle the turnoff. Don't we need also to set lsp-ui-doc-enable to nil to avoid starting ui-doc in the new buffers?

(lsp-ui-doc--callback (lsp-request "textDocument/hover" (lsp--text-document-position-params))
(or (bounds-of-thing-at-point 'symbol) (cons (point) (1+ (point))))
(current-buffer)))
(when lsp-ui-doc-mode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will prevent the user from using ui-doc without enabling lsp-ui-doc, right? I think this is the goal of lsp-ui-doc-show

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it make sense it shows only when the lsp-ui-doc is enabled. So yes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of lsp-ui-doc-show is to be used when automatic lsp-ui-doc is off. This change will allow showing the doc manually only when the automatic popup is enabled which does not make sense.

@jcs090218
Copy link
Member Author

jcs090218 commented Jun 18, 2021

Ok, seems like the global mode will handle the turnoff. Don't we need also to set lsp-ui-doc-enable to nil to avoid starting ui-doc in the new buffers?

No, I think the minor-mode will still enable if user have set it up in the config. For instance,

(global-lsp-ui-doc-mode 1)

Then it will automatically be turn on no matter what, and I think that's the default action from Emacs. If user want to disable lsp-ui completely then it will have to evaluate expression manually.

(setq lsp-ui-doc-enable nil)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants