The Wayback Machine - https://web.archive.org/web/20210118083505/https://github.com/emacs-lsp/lsp-mode/pull/2531
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

Update lsp-tramp-connection #2531

Open
wants to merge 2 commits into
base: master
from

Conversation

@petr-tik
Copy link

@petr-tik petr-tik commented Jan 16, 2021

Copy the implementation that Michael Albinus suggested when
graciously helping people understand why lsp wasn't working over tramp.

https://lists.gnu.org/archive/html/emacs-devel/2020-12/msg00896.html

This replaces start-file-process-shell-command with make-process
and moving from binary encoding to none.

One disadvantage is, that direct asynch processes work only with the
upcoming Tramp 2.5 (that's already in Emacs master), and it works only
if the asynchronous process does not require password handling. The
latter is true, if your ssh authentication is based on keys, or if you
use Tramp's control master arguments (enabled by default).

Added a Troubleshooting section to the remote doc with examples
of the errors. Suggested solution and tips to debug other
remote LSP errors.

Copy the implementation that Michael Albinus suggested when
graciously helping people understand why lsp wasn't working over tramp.

https://lists.gnu.org/archive/html/emacs-devel/2020-12/msg00896.html

replacing start-file-process-shell-command with make-process and moving
from binary encoding to none.

> One disadvantage is, that direct asynch processes work only with the
upcoming Tramp 2.5 (that's already in Emacs master), and it works only
if the asynchronous process does not require password handling. The
latter is true, if your ssh authentication is based on keys, or if you
use Tramp's control master arguments (enabled by default).
@petr-tik petr-tik force-pushed the petr-tik:tramp_connection_compliant_with_2_5 branch 3 times, most recently from 97052ae to 24210cb Jan 16, 2021
In the author's experience, tramp 2.5.0-pre worked fine with old and new
implementation of lsp-tramp-connection. From the bug reports, reddit
messages and other information, it looked like emacs27.1 + tramp
>2.5.0-pre works fine

Added a Troubleshooting section to the remote doc with examples
of the errors. Suggested solution and tips to debug other
remote LSP errors.
@petr-tik petr-tik force-pushed the petr-tik:tramp_connection_compliant_with_2_5 branch from 24210cb to fe45fe9 Jan 16, 2021
Copy link
Member

@yyoncho yyoncho left a comment

Looks good.

(defvar tramp-connection-properties)
(when (version< "2.5.0-pre" tramp-version)
(lsp-warn
"Your tramp version - %s - might fail to work with remote LSP. Update to tramp-2.5 for tested reliability improvements"

This comment has been minimized.

@yyoncho

yyoncho Jan 17, 2021
Member

can you change the wording to mention that you can install that version from elpa?

;; wrap with stty to disable converting \r to \n
(let* ((final-command (lsp-resolve-final-function
local-command))
(_stderr (or (when generate-error-file-fn

This comment has been minimized.

@yyoncho

yyoncho Jan 17, 2021
Member

this should be appended to the command. Otherwise, the stderr will end up on stdout. (Unless this is fixed in the latest tramp).

This comment has been minimized.

@petr-tik

petr-tik Jan 17, 2021
Author

what do you mean by this here?

This comment has been minimized.

@yyoncho

yyoncho Jan 18, 2021
Member

_stderr

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.