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 remote support #1922
base: master
Are you sure you want to change the base?
Add remote support #1922
Conversation
I am a newbie to this project. |
In my local test, these modification were necessary even for master branch.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like it will run correctly, but the user is likely to have Emacs freeze for several seconds during their use with connections with long delays ("remote hosts that are far away")
flycheck.el
Outdated
This function is a wrapper for `file-local-name' function. | ||
if `file-local-name' is available, this function invokes it only if | ||
FILE is not nil. | ||
If FILEL is nil or `file-local-name' is not available, returns FILE." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: FILEL
FWIW, I cloned this locally and rebased on master (there were no conflicts), and this seems to be working well. |
c87ee3b
to
23c37bf
Compare
I'm sorry. I failed to respond to @FelipeLema's advice to fix the typo. Now, I will check the difference between the two branches (both in terms of source code and behavior). |
23c37bf
to
869f6ea
Compare
@yfyyfy Are you still interested in getting this to the finish line? |
I think this might have been fixed with the v33 release, although I'm not sure exactly what changed. Can someone else confirm it's also working for them now?
|
I take that back, when I enable I rebased this PR on master and resolved the merge conflicts (which were just dash.el changes), and it still doesn't work :( @yfyyfy, does this still work for you? |
This patch does three things: * Update `flycheck-default-executable-find` to search remote hosts when `default-directory` is a remote directory. See also the documentation for [executable-find](https://www.gnu.org/software/emacs/manual/html_node/elisp/Locating-Files.html). * Change uses of `call-process` and `start-process` to `start-file-process` which is the same as these two functions, but starts the process on a remote host when `default-directory` is a remote directory. See also the documentation for [start-file-process](https://www.gnu.org/software/emacs/manual/html_node/elisp/Asynchronous-Processes.html). * Passes the local part of `buffer-file-name` to the compile command, e.g. strips `/ssh:10.0.0.42`, since the checker is run on the remote machine where the file name must be local. See also the documentation for [file-local-name](https://www.gnu.org/software/emacs/manual/html_node/elisp/Magic-File-Names.html).
Such checkers (including python-pycompile) use a temp directory. For executing flycheck on remote files, temp directories should be created on remote.
Some checkers (including python-flake8 and python-pycompile) use flycheck-parse-error-with-patterns as their error-parser. This commit changes `flycheck-parse-error-with-patterns' behavior so that it always returns `flycheck-error' struct with its :filename slot in the form of local path on remote files (i.e., without "/method:user@host:" part).
Some checkers set remote path or filename (without directory path) to `flycheck-error' struct's :filename slot. For example, on remote files, python-pylint sets remote path (i.e., with "/method:user@host:" part). python-flake8 before the previous comiit also sets remote path. py-compile sets filename only. In all these cases, after processing with `flycheck-fill-and-expand-error-file-names', :filename slot of `flycheck-error' struct was remote path. This commit changes `flycheck-fill-and-expand-error-file-names' behavior so that it always returns `flycheck-error' struct with its :filename slot in the form of local path on remote files (i.e., without "/method:user@host:" part).
Some checkers (including python-pylint) use `flycheck-save-buffer-to-temp' to create temp file. For executing flycheck on remote files, their corresponding temp files should be created on remote (therefore, they should be specified as remote path, i.e., with "/method:user@host:" part). On the other hand, the return value of `flycheck-save-buffer-to-temp' should be local path (i.e., without "/method:user@host:" part) because the value is used as an argument to checker's executable.
Flycheck uses `flycheck-locate-config-file-functions' to locate config file. Its value is the list of functions below: - flycheck-locate-config-file-by-path - flycheck-locate-config-file-ancestor-directories - flycheck-locate-config-file-home Each locate function should use remote path (i.e., with "/method:user@host:" part) to locate a config file. When the return value is used as an argument to a checker's executable, the path should be converted to local path (i.e., without "/method:user@host:" part). Tested with python-flake8 and python-pylint.
The original version used stdin instead of a temporary file (as with javascript-eslint). However, behavior of eslint and stylelint is different in terms of stdin processing. Corresponding CLI commands for both tools are: eslint: eslint --format=json --stdin --stdin-filename FILENAME-TO-ASSIGN-TO-STDIN stylelint: stylelint --formatter json --stdin-filename FILENAME-TO-ASSIGN-TO-STDIN or stylelint --formatter json --stdin --stdin-filename FILENAME-TO-ASSIGN-TO-STDIN eslint waits for stdin EOF and keeps itself alive, while stylelint does not wait and may exit before input comes in. When stylelint does not receive stdin, it emits its usage message (when --stdin is not specified) or outputs "empty source" lint message (when --stdin is specified). For local files, the original version was able to pass input with `flycheck-process-send-buffer' before stylelint process exited. For remote files, stylelint process exited before `flycheck-process-send-buffer' called. This commit changes *-styelint commands so that they use a temporary file (instead of stdin) as their input to avoid the above situation on remote.
To add support for remote files, some occurrences of filenames had to be converted to local path on remote (i.e., without "/method:user@host:" part). However, these filenames could be nil, while file-local-name signals an error when its argument is nil. This led to test failure for scss-lint and others. (Test command: "make LANGUAGE=scss integ") This commit adds a wrapper function of file-local-name that accepts nil as its argument and return it as it is.
Replace all occurrences is excessive. For example, arguments to checker executables are not expected to be nil and thus file-local-name will suffice. However, here, all occurrences are replaced just in case.
In `flycheck-locate-config-file-home', file-remote-p is used to determine if the context is local or remote. Because file-remote-p does not accept nil, (file-remote-p (buffer-file-name)) may cause an error. It led to unit test failure: FAILED flycheck-locate-config-file/existing-file-in-home-directory FAILED flycheck-locate-config-file/existing-file-in-parent-directory FAILED flycheck-locate-config-file/not-existing-file This commit adds a nil-check for buffer-file-name. When buffer-file-name returns nil, the context is assumed to be local.
integ test for Emacs25.3 failed due to undefined functions. Error: the following functions are not known to be defined: file-local-name, temporary-file-directory This commit fixes the problem by reverting to the original codes when the functions are not available.
@bbatsov |
@gsingh93 I rebased the code on master (bec59ee) locally and checked if
By the way, as you might know, remote files are explicitly excluded from the targets of |
The outputs of flake8 are parsed by flycheck-parse-with-patterns, which does not take ASCII escape sequences into account. Therefore, flake8 should return simple texts (without escape sequences).
869f6ea
to
d766cd5
Compare
I found a solution for this.
In my case, remote flake8 returned colored output. |
Add support for remote hosts via TRAMP.
In response to #1816,
based on #1842
Enable the following checkers to work on remote files:
Probably further additional changes may be necessary for some (or majority) of the others checkers.
Remote support works only with Emacs >=27.1.
One function is added: file-local-name-pass-nil.
It is not prefixed with "flycheck-" as with the other functions added in #1842.
Travis CI may pass with the following modification:
Access to repository (and thus cask installation) fail.
yfyyfy@09271d2
Installation of bazel-mode by cask fails.
yfyyfy@4dc94e2
It prompts for "Scheme implementation" and fails in Travis CI.
yfyyfy@9faddc0
(The above references are the last three commits of https://github.com/yfyyfy/flycheck/tree/add-remote-support-tweak-test)