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

didChange with stdin = false specifies the filename of the (potentially unsaved) file #59

Open
aktau opened this issue Jul 8, 2020 · 3 comments

Comments

@aktau
Copy link

aktau commented Jul 8, 2020

Hi! My use case is running shellcheck(1) with efm-langserver (using the nvim-lsp client). I noticed that after I made a change to my document the returned diagnostics did not match the new content, but the old one (reference: nvim-lua/diagnostic-nvim#40).

The reason is: nvim-lsp sends a textDocument/didChange whenever the user makes a change. This does not mean that the document is saved.

My configuration is essentially copied from the README:

# Cut down version of https://github.com/mattn/efm-langserver.
version: 2

tools:
  sh-shellcheck: &sh-shellcheck
    lint-command: 'shellcheck -f gcc -x'
    lint-formats:
      - '%f:%l:%c: %trror: %m'
      - '%f:%l:%c: %tarning: %m'
      - '%f:%l:%c: %tote: %m'

languages:
  sh:
    - <<: *sh-shellcheck

I traced the shellcheck invocation made by efm-langserver before and after the edit of a shell file:

shellcheck -f gcc -x /home/aktau/tmp/glop.sh # When opening file
shellcheck -f gcc -x /home/aktau/tmp/glop.sh # After editing, but the file isn't saved yet so this is identical to before.

Reading the code I also noticed that f.Text is only used when stdin is false. This means that I'd need to use shellcheck in stdin mode to get this to work. A couple of notes:

  • It would be better to switch the shellcheck example in the README to use stdin (-).
  • I think it would be a good idea to check how other language servers fix this (or, for example, syntastic, which also uses shellcheck). For example, it may make sense to just pass /dev/stdin instead of the filename for all tools on *nix operating systems. Or to create a temporary file (some tools may require a seekable file, or for Windows this may be necessary).
aktau added a commit to aktau/dotfiles that referenced this issue Jul 8, 2020
@mattn
Copy link
Owner

mattn commented Jul 8, 2020

Okay, please send pull-request.

@aktau
Copy link
Author

aktau commented Jul 8, 2020

I'll likely only have time for something small. But also: any thoughts on the last paragraph?

@mattn
Copy link
Owner

mattn commented Jul 10, 2020

There are not any meaning about that current README.md does not have lint-stdin in shellcheck section. So Iet's change it to use.

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

No branches or pull requests

2 participants