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

Kill remote process #953

Merged
merged 9 commits into from
Jun 9, 2024

Conversation

werhner
Copy link
Contributor

@werhner werhner commented May 23, 2024

  1. add option lsp-bridge-remote-enable-kill-process, which can kill remote lsp-bridge process when Emacs exit
  2. add option acm-backend-ctags-max-candidates
  3. add command lsp-bridge-ctags-find-def-at-point, which uses xref as front-end.

Comment on lines 163 to 168
(let ((end-position (line-end-position)))
(let ((start-position (line-beginning-position))
(middle-position (point)))
(forward-line (- (plist-get candidate-info :line) (count-lines (point-min) (line-beginning-position))))
(if preview
(acm-preview-create-overlay (point) end-position (plist-get candidate-info :label))
(delete-region (point) end-position)
(acm-preview-create-overlay start-position middle-position (plist-get candidate-info :label))
(delete-region start-position middle-position)
Copy link
Owner

Choose a reason for hiding this comment

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

Above code fix some bug?

lsp-bridge.el Outdated
Comment on lines 2149 to 2180
(defface lsp-bridge-tag-annotation-face
'((((background light))
:foreground "#666666" :slant italic)
(t
:foreground "#c0c0c0" :slant italic))
"Face used for annotations when presenting a tag.
Annotations include kind, type, etc.")

(defun lsp-bridge-xref--make-object (tag)
"Make xref object of TAG."
(let* ((path (plist-get tag :ext-abspath))
(line (plist-get tag :line))
(str (plist-get tag :str))
(annotation (plist-get tag :annotation)))
(xref-make
(if annotation
(concat (propertize (concat "(" annotation ") ") 'face 'lsp-bridge-tag-annotation-face) str)
str)
(xref-make-file-location path line 0))))

(defun lsp-bridge-xref-callback (tags)
(let ((fetcher (lambda () (mapcar #'lsp-bridge-xref--make-object tags))))
(xref-show-xrefs fetcher nil)))

(defun lsp-bridge-ctags-find-def-at-point ()
(interactive)
(if (lsp-bridge-is-remote-file)
(lsp-bridge-remote-send-func-request "ctags_find_def"
(list
(symbol-at-point)
(file-local-name (buffer-file-name))))
(lsp-bridge-call-async "ctags_find_def" (symbol-at-point) (buffer-file-name))))
Copy link
Owner

Choose a reason for hiding this comment

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

Those code should move to file acm-backend-ctags.el , and rename with acm-backend-ctags-*

Comment on lines +836 to +840
def close_client(self):
for client in self.client_dict.values():
if hasattr(client, "kill_lsp_bridge_process"):
client.kill_lsp_bridge_process()

Copy link
Owner

Choose a reason for hiding this comment

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

Why not add close_client code to function "cleanup" ?

lsp-bridge.el Outdated
Comment on lines 1137 to 1138
(when lsp-bridge-remote-enable-kill-process
(lsp-bridge-call-sync "close_client"))
Copy link
Owner

Choose a reason for hiding this comment

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

Shoud remove those code if code of close_client move to "cleanup" function.

And don't use lsp-bridge-call-sync, it will block Emacs.

lsp-bridge.el Outdated
Comment on lines 1006 to 1010
(defun lsp-bridge-call-sync (method &rest args)
"Call Python EPC function METHOD and ARGS synchronously."
(lsp-bridge-deferred-chain
(lsp-bridge-epc-call-sync lsp-bridge-epc-process (read method) args)))

Copy link
Owner

Choose a reason for hiding this comment

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

Don't use this function, it will block Emacs

@manateelazycat
Copy link
Owner

Thanks for patch, I have review code, need adjust. ;)

@werhner
Copy link
Contributor Author

werhner commented May 24, 2024

Thanks for the review, I'll try to adjust these in my spare time.

@manateelazycat
Copy link
Owner

@werhner Please let me know if this patch can be merged, thank you

"Maximal number of candidate of menu."
:type 'integer
:group 'acm-backend-ctags)
(require 'xref)
Copy link
Owner

Choose a reason for hiding this comment

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

Duplicate code?

Comment on lines +842 to 843
self.close_client()
"""Do some cleanup before exit python process."""
Copy link
Owner

Choose a reason for hiding this comment

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

close_client should move to below of function comment

@manateelazycat
Copy link
Owner

@werhner Please adjust code when you have time. I will merge your patch as soon as possible. Thank you.

@manateelazycat manateelazycat merged commit 9f79295 into manateelazycat:master Jun 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants