manateelazycat / lsp-bridge

A blazingly fast LSP client for Emacs
GNU General Public License v3.0
1.42k stars 205 forks source link

lsp-bridge-open-remote-file功能异常 #854

Closed norris-young closed 7 months ago

norris-young commented 7 months ago

我发现lsp-bridge-open-remote-file好像有些异常,我这里尝试用这个command时,文件可以被打开,但是不会切到这个文件的buffer,相应的一些lsp-bridge的local varaible(比如lsp-bridge-remote-file-flag,也没有被配置).我debug了一下发现在(funcall mode)的时候lsp-bridge-open-remote-file--response就退出了.更深的原因我还没有定位到.也不清楚是否和我的个人配置有关系,因为如果我不去做(funcall mode)那么后半部分会正常执行,也会切到open的buffer中, 不知道是不是我的某些mode hook有冲突引起的. 我之前没用过这个功能,我都是通过tramp使用remote功能.只是最近提patch,我也测试了一下这个command,但是好像不太正常,不过应该和我的patch没关系.不知道这一组open-remote-file save-file之类的功能的应用场景是什么. 另外,如果tramp已经支持的比较好的情况下,还有必要提供这个独立的remote-file功能么? tramp已经是emacs的内置package了, 应该也不会出现有人没有tramp,而无法使用remote功能的情况.

(defun lsp-bridge-open-remote-file--response(tramp-method user host port path content position)
  (let ((buf-name (format "[LBR] %s" (file-name-nondirectory path))))
    (lsp-bridge-define--jump-record-postion)

    (with-current-buffer (get-buffer-create buf-name)
      (text-mode)

      (read-only-mode -1)
      (erase-buffer)
      (insert (lsp-bridge-decode-base64 content))
      (goto-char (point-min))

      (add-hook 'kill-buffer-hook 'lsp-bridge-remote-kill-buffer nil t)

      (let ((mode (lsp-bridge-get-mode-name-from-file-path path)))
        (when mode
          (let ((lsp-bridge-remote-file-flag t)
                (lsp-bridge-remote-file-tramp-method tramp-method)
                (lsp-bridge-remote-file-user user)
                (lsp-bridge-remote-file-host host)
                (lsp-bridge-remote-file-port port)
                (lsp-bridge-remote-file-path path))
            (funcall mode)))))

    (switch-to-buffer buf-name)

    (unless (equal position (list :line 0 :character 0))
      (setq-local lsp-bridge-mark-ring (append (list position-before-jump) mark-ring))

      (lsp-bridge-define--jump-flash position))

    (setq-local lsp-bridge-remote-file-flag t)
    (setq-local lsp-bridge-remote-file-tramp-method tramp-method)
    (setq-local lsp-bridge-remote-file-user user)
    (setq-local lsp-bridge-remote-file-host host)
    (setq-local lsp-bridge-remote-file-port port)
    (setq-local lsp-bridge-remote-file-path path)

    ;; Always enable lsp-bridge for remote file.
    ;; Remote file can always edit and update content even some file haven't corresponding lsp server, such as *.txt
    (lsp-bridge-mode 1)))
manateelazycat commented 7 months ago

tramp只是提供文件路径,lsp-bridge的远程补全是完全不用tramp的同步机制的。

我认为tramp本身的实现就非常垃圾,依然有很多人不用tramp,太卡了。

lsp-bridge唯一支持tramp的目的是方便很多人远程访问目录,方便打开文件

manateelazycat commented 7 months ago

我的猜测是跟你最近最大的那个补丁有关系,你没有考虑非tramp的场景。

norris-young commented 7 months ago

我的猜测是跟你最近最大的那个补丁有关系,你没有考虑非tramp的场景。

最大的那个补丁 我什么代码都没改 只是移动了一下代码的位置 分类摆放 我昨天提交的这个PR才是第一个改动较大的patch

norris-young commented 7 months ago

分析了一下

  1. (funcall mode)的问题是我本地的一些mode-hook会获取(buffer-file-name),但是lsp-bridge-open-remote-file打开的文件(buffer-file-name)nil,去掉我的mode-hook可以正常打开文件.
  2. 我又测试了一下,lsp-bridge-find-deflsp-bridge-find-references. 之前的一个patch里因为也调用了(buffer-file-name),所以会有问题,需要修改. 由此可以知道lsp-bridge中所有调用(buffer-file-name)的地方都需要考虑open-remote-file的情况
    (defun lsp-bridge-call-file-api (method &rest args)
    (if (file-remote-p (buffer-file-name))
      (if (lsp-bridge-is-remote-file)
          (lsp-bridge-remote-send-lsp-request method args)
        (message "[LSP-Bridge] remote file \"%s\" is updating info... skip call %s."
                 (buffer-file-name) method))
  3. lsp-bridge-enable-with-tramp需要设置为nil,否则在lsp-bridge-define--jump中会通过tramp打开会出错.
  4. lsp-bridge-find-references找到的reference并没有办法跳转,我看lsp-bridge-ref-open-file实现里并没有调用open-remote-file, 直接调用的find-file,所以这些reference如果没有tramp支持并没有办法跳转.那么open-remote-file这一套机制是否支持lsp-bridge的全功能呢?还是仅支持find-define呢?
  5. 另外open-remote-file在解析远程文件名的时候,不支持包含'.'的用户名,re pattern也需要修正.
    
    def is_valid_ip_path(ssh_path):
    """Check if SSH-PATH is a valid ssh path."""
    pattern = r"^(?:([a-z_][a-z0-9_-]*)@)?((?:[0-9]{1,3}\.){3}[0-9]{1,3})(?::(\d+))?:~?(.*)$"
    match = re.match(pattern, ssh_path)
    return match is not None

def split_ssh_path(sshpath): """Split SSH-PATH into username, host, port and path.""" pattern = r"^(?:([a-z][a-z0-9_-])@)?((?:[0-9]{1,3}.){3}[0-9]{1,3})(?::(\d+))?:?(.)$"


6.  我个人是感觉这一套open-remote-file的机制好像有很多限制,设计实现也还并不完善.就如你说的,tramp只是用来打开文件而已,lsp-bridge工作的过程中大部分情况并不通过tramp,编辑时应该也并不会受到tramp低性能的影响.所以我个人还是倾向于通过tramp来访问文件,其通用性和兼容性还是要比open-remote-file要好很多,比如支持使用ssh config里配置服务器alias,打开文件时也支持远程路径补全等等,这些好像open-remote-file目前还没有支持. 仅个人看法.
norris-young commented 7 months ago
2. 我又测试了一下,`lsp-bridge-find-def`和`lsp-bridge-find-references`. 之前的一个patch里因为也调用了`(buffer-file-name)`,所以会有问题,需要修改. 由此可以知道lsp-bridge中所有调用`(buffer-file-name)`的地方都需要考虑open-remote-file的情况
(defun lsp-bridge-call-file-api (method &rest args)
  (if (file-remote-p (buffer-file-name))
      (if (lsp-bridge-is-remote-file)
          (lsp-bridge-remote-send-lsp-request method args)
        (message "[LSP-Bridge] remote file \"%s\" is updating info... skip call %s."
                 (buffer-file-name) method))

lsp-bridge-call-file-api的问题已经提交PR #855

manateelazycat commented 7 months ago

春节自驾路上,没法一一回复,你说的没问题,我只是觉得不能只有tramp这一种路径,最起码我自己是完全不用tramp的

norris-young commented 7 months ago

春节自驾路上,没法一一回复,你说的没问题,我只是觉得不能只有tramp这一种路径,最起码我自己是完全不用tramp的

不急 猫大空了再沟通

manateelazycat commented 7 months ago

我简单回复一下哈, 欢迎进一步讨论:

  1. 社区之间修复了很多 issue 都跟 buffer-file-name 这个函数返回的buffer name带text prop相关, 所以所有用到 (buffer-file-name) 的地方都应该用 lsp-bridge-get-buffer-file-name-text 或者 lsp-bridge-buffer-file-name 替代, 避免因为属性导致各种奇怪的bug
  2. lsp-bridge-find-def和lsp-bridge-find-references只测试过文件打开以后, 支持远程文件的跳转, 但是你说的文件正在打开中的情况确实没有处理
  3. lsp-bridge-define--jump 和 tramp 的情况我没有测试过, 因为我平常不用 tramp
  4. open-remote-file 的机制我觉得遇到什么不方便就改哪个地方吧,因为使用是最好的测试, 不使用直接按照理论方法去改, 容易出现bug
  5. 对的, tramp目前看最重要的使用方式是和dired结合, 用户可以先通过 dired 访问远程的目录, 然后通过 tramp 的机制快速打开文件, lsp-bridge的整个补全机制和tramp是不相关的, 我原来准备直接在我的 eaf-file-manager 文件管理器中添加远程目录的访问, 这样就可以完全不用 tramp, 但是后面考虑到不是所有用户都用 tramp, 所以我也倾向于在远程目录访问上对 tramp 提供兼容性支持, 毕竟不是所有emacser的习惯都和我一样
  6. 路径补全的后端其实是可以支持远程文件补全的, 因为 lsp-bridge 的远程补全思想和 VSCode 是类似的, 所有补全的计算都在服务器里, 包括 lsp-bridge Python 部分和 LSP Server 或 acm 补全后端, 得到数据后再传输补全候选词到本地进行前端绘制
manateelazycat commented 7 months ago

我刚刚看了代码, 远程路径补全已经有代码了, 因为我最开始写远程补全的时候测试过 (但是那时候没有测试过 tramp), 如果你那边不能工作, 可以查看一下

https://github.com/manateelazycat/lsp-bridge/blob/4bbaad092e6f8b982dfdf9aa68465a6e3dcca818/lsp-bridge.el#L1589-#L1593

今天还要开长途, 估计回复不会那么及时。

norris-young commented 7 months ago

我刚刚看了代码, 远程路径补全已经有代码了, 因为我最开始写远程补全的时候测试过 (但是那时候没有测试过 tramp), 如果你那边不能工作, 可以查看一下

https://github.com/manateelazycat/lsp-bridge/blob/4bbaad092e6f8b982dfdf9aa68465a6e3dcca818/lsp-bridge.el#L1589-#L1593

今天还要开长途, 估计回复不会那么及时。

嗯 在文件中编辑时补全路径我知道是支持的 我说的路径补全是指在打开文件的时候可以像打开本地目录一样按tab补全 lsp-bridge-open-remote-file打开某个服务器文件时目前应该得自己输入完整的路径和文件名 tramp是可以的,所以这是我个人使用tramp的原因之一

2. lsp-bridge-find-def和lsp-bridge-find-references只测试过文件打开以后, 支持远程文件的跳转, 但是你说的文件正在打开中的情况确实没有处理

lsp-bridge-find-references目前可以正常跳转吗?我这里不使用tramp好像是不行.可以得到references列表,但是没法跳转到对应文件(未打开的文件). 我看lsp-bridge-ref-open-file的实现好像也没有使用(lsp-bridge-call-async "open_remote_file")而是直接用的find-file.

lsp-bridge-define--jump里会判断lsp-bridge-enable-with-tramp调用(lsp-bridge-call-async "open_remote_file"),所以lsp-bridge-find-def是不使用tramp也是可以正常跳转到未打开的定义文件的.

(defun lsp-bridge-ref-open-file (&optional stay)
  (interactive)
  (let* ((match-file (lsp-bridge-ref-get-match-file))
         (match-line (lsp-bridge-ref-get-match-line))
         (match-column (lsp-bridge-ref-get-match-column))
         (match-buffer (lsp-bridge-ref-get-match-buffer match-file))
         in-org-link-content-p
         ref-buffer-window)
    (save-excursion
      (let ((inhibit-message t))
        ;; Try fill variables when in org file.
        (lsp-bridge-ref-move-to-column match-column)
        (setq in-org-link-content-p
              (and (lsp-bridge-ref-is-org-file match-file)
                   (lsp-bridge-ref-in-org-link-content-p)))
        ;; Open file in other window.
        ;; Note, don't use `find-file-other-window', it will failed if path is tramp path that start with /sudo:root
        (if (and lsp-bridge-ref-open-file-in-request-window
                 ;; check if the window which requested search has beed deleted.
                 (window-valid-p lsp-bridge-ref-request-search-window))
            (select-window lsp-bridge-ref-request-search-window)
          (other-window 1))
        (find-file match-file)
        ;; Add to temp list if file's buffer is not exist.
        (unless match-buffer
          (add-to-list 'lsp-bridge-ref-temp-visit-buffers (current-buffer)))
        ;; Jump to match point.
        ;; We use `ignore-errors' to make sure cursor will back to lsp-bridge-ref buffer
        ;; even target line is not exists in search file (such as delete by user).
        (ignore-errors
          (lsp-bridge-ref-move-to-point match-line match-column)
          (when (lsp-bridge-ref-is-org-file match-file)
            ;; Expand org block if current file is *.org file.
            (org-reveal)
            ;; Jump to link beginning if keyword in content area.
            (when in-org-link-content-p
              (search-backward-regexp "\\[\\[" (line-beginning-position) t))
            )))
      ;; Flash match line.
      (lsp-bridge-ref-flash-line))
    (unless stay
      ;; Keep cursor in search buffer's window.
      (setq ref-buffer-window (get-buffer-window lsp-bridge-ref-buffer))
      (if ref-buffer-window
          (select-window ref-buffer-window)
        (if lsp-bridge-ref-delete-other-windows
            (delete-other-windows))
        ;; Split window and select if color-buffer is not exist in windows.
        (split-window)
        (other-window 1)
        (switch-to-buffer lsp-bridge-ref-buffer)))
    ;; Ajust column position.
    (lsp-bridge-ref-move-to-column match-column)
    ))

(defun lsp-bridge-define--jump (filename filehost position)
  (let (position-before-jump)
    (lsp-bridge-define--jump-record-postion)

    (if (or (string-equal filehost "") lsp-bridge-enable-with-tramp)
        (progn
          (setq filename (concat (cdr (assoc filehost lsp-bridge-tramp-alias-alist)) filename))
          (let ((match-window (lsp-bridge-find-window-match-filename filename)))
            (if (and lsp-bridge-find-def-select-in-open-windows
                     match-window)
                ;; Try select to window if `lsp-bridge-find-def-select-in-open-windows' is non-nil.
                (select-window match-window)
              ;; Jump to define.
              ;; Show define in other window if `lsp-bridge-jump-to-def-in-other-window' is non-nil.
              (if lsp-bridge-jump-to-def-in-other-window
                  (find-file-other-window filename)
                (find-file filename))
              ))

          ;; Init jump history in new buffer.
          (setq-local lsp-bridge-mark-ring (append (list position-before-jump) mark-ring))

          (lsp-bridge-define--jump-flash position))
      (lsp-bridge-call-async "open_remote_file" (format "%s:%s" filehost filename) position))
    ))
norris-young commented 7 months ago
5. 另外open-remote-file在解析远程文件名的时候,不支持包含'.'的用户名,re pattern也需要修正.
def is_valid_ip_path(ssh_path):
    """Check if SSH-PATH is a valid ssh path."""
    pattern = r"^(?:([a-z_][a-z0-9_-]*)@)?((?:[0-9]{1,3}\.){3}[0-9]{1,3})(?::(\d+))?:~?(.*)$"
    match = re.match(pattern, ssh_path)
    return match is not None

def split_ssh_path(ssh_path):
    """Split SSH-PATH into username, host, port and path."""
    pattern = r"^(?:([a-z_][a-z0-9_-]*)@)?((?:[0-9]{1,3}\.){3}[0-9]{1,3})(?::(\d+))?:?(.*)$"

这个问题提交了一个PR #856

manateelazycat commented 7 months ago

lsp-bridge-find-references

lsp-bridge-find-references 最开始是可以的, 欢迎提交PR

manateelazycat commented 7 months ago
5. 另外open-remote-file在解析远程文件名的时候,不支持包含'.'的用户名,re pattern也需要修正.
def is_valid_ip_path(ssh_path):
    """Check if SSH-PATH is a valid ssh path."""
    pattern = r"^(?:([a-z_][a-z0-9_-]*)@)?((?:[0-9]{1,3}\.){3}[0-9]{1,3})(?::(\d+))?:~?(.*)$"
    match = re.match(pattern, ssh_path)
    return match is not None

def split_ssh_path(ssh_path):
    """Split SSH-PATH into username, host, port and path."""
    pattern = r"^(?:([a-z_][a-z0-9_-]*)@)?((?:[0-9]{1,3}\.){3}[0-9]{1,3})(?::(\d+))?:?(.*)$"

这个问题提交了一个PR #856

已经合并了,感谢补丁

norris-young commented 7 months ago

lsp-bridge-find-references

lsp-bridge-find-references 最开始是可以的, 欢迎提交PR

好的 等我这边的patch都提交完 我可以看下这个怎么实现

norris-young commented 7 months ago

863

支持不使用tramp跳转到references文件 因为我用tramp,这个功能后续我也不会常用,所以猫大有空的话,也测试下,再看看实现合不合理.

norris-young commented 7 months ago

863 支持不使用tramp跳转到references文件 因为我用tramp,这个功能后续我也不会常用,所以猫大有空的话,也测试下,再看看实现合不合理.

发现2个问题:

  1. tramp host name解析的地方没考虑纯ip的情况,提了一个fix PR #864

  2. file-remote-p只有在tramp-syntaxsimplified情况下这里才能判断成功,这个估计得改成string-match-p,等晚上回来我再看看 https://github.com/manateelazycat/lsp-bridge/blob/dda0d227d071be0406cd1292ca53271b6a65c8ea/lsp-bridge-ref.el#L853C51-L853C79

norris-young commented 7 months ago
2. `file-remote-p`只有在`tramp-syntax`为`simplified`情况下这里才能判断成功,这个估计得改成`string-match-p`,等晚上回来我再看看
   https://github.com/manateelazycat/lsp-bridge/blob/dda0d227d071be0406cd1292ca53271b6a65c8ea/lsp-bridge-ref.el#L853C51-L853C79

865 #866 应该是最后一波PR了

manateelazycat commented 7 months ago
2. `file-remote-p`只有在`tramp-syntax`为`simplified`情况下这里才能判断成功,这个估计得改成`string-match-p`,等晚上回来我再看看
   https://github.com/manateelazycat/lsp-bridge/blob/dda0d227d071be0406cd1292ca53271b6a65c8ea/lsp-bridge-ref.el#L853C51-L853C79

865 #866 应该是最后一波PR了

感谢, 已经合并了。

norris-young commented 7 months ago

最新的master我本地测试find.define, find-ref, 补全功能应该都没问题了 最开始其实我的问题只是 #831 ,应该是加一个心跳机制就解决的.没想到一路提了这么多patch 感谢猫大 龙年龙腾龘龘!

manateelazycat commented 7 months ago

辛苦啦,新年快乐,万事顺利