prabirshrestha / vim-lsp

async language server protocol plugin for vim and neovim
MIT License
3.14k stars 305 forks source link

Rename not always applied to multi-document projects #417

Closed clason closed 5 years ago

clason commented 5 years ago

This one is a bit difficult to describe since it most often occurs in larger projects, but I found one instance in a minimal setup. The language server is https://github.com/latex-lsp/texlab (for LaTeX, a macro-based markup language used to write (not only) scientific publications), but I already verified that this server works correctly in VS Code. Consider the following setup of three documents:

\begin{document} \include{test1} \include{test2} \end{document}

* `test1.tex` containing
```tex
\section{Section 1}\label{sec:1}

This section continues from Section \ref{sec:1}

(If compiled using `pdflatex test.tex` (twice!), it generates a `test.pdf` containing  
> 1. Section 1
> 2. Section 2
> This section continues from Section 1.

Texlab offers a Rename action that allows renaming `label` and `ref` across a whole project, but this doesn't work from `LspRename` in all cases:

1. Opening `test2.tex`, putting the cursor on `sec:1` and calling `:LspRename` allows replacing this text with, say  `sec:one`. Upon pressing enter, this instance is renamed, `test1.tex` is opened in a new buffer, and `label{sec:1}` is renamed to `label{sec:one}`. This is as expected.

2. But opening `test1.tex` and doing the same (here, in the `label`), the rename is performed only in the current file, but `test2.tex` is not opened. (As I wrote, this *does* work with the same server in VS Code).

In case it helps, I attach two logs, both starting from invoking the Rename action and ending when the vi instance is closed (since I didn't know where the relevant bits end).

1. Working case:

Wed 26 Jun 2019 10:56:22 AM CEST:["--->", 5, "texlab", {"method": "textDocument/rename", "on_notification": "---funcref---", "params": {"newName": "sec:one", "textDocument": {"uri": "file:///scratch/test/test2.tex"}, "position": {"character": 45, "line": 2}}}] Wed 26 Jun 2019 10:56:22 AM CEST:["<---", 5, "texlab", {"response": {"id": 6, "jsonrpc": "2.0", "result": {"changes": {"file:///scratch/test/test.tex": [], "file:///scratch/test/test1.tex": [{"range": {"end": {"character": 31, "line": 0}, "start": {"character": 26, "line": 0}}, "newText": "sec:one"}], "file:///scratch/test/test2.tex": [{"range": {"end": {"character": 46, "line": 2}, "start": {"character": 41, "line": 2}}, "newText": "sec:one"}], "file:///scratch/test/test3.tex": []}}}, "request": {"id": 6, "jsonrpc": "2.0", "method": "textDocument/rename", "params": {"newName": "sec:one", "textDocument": {"uri": "file:///scratch/test/test2.tex"}, "position": {"character": 45, "line": 2}}}}] Wed 26 Jun 2019 10:56:22 AM CEST:["s:build_cmd", "keepjumps keepalt edit /scratch/test/test1.tex | execute 'keepjumps normal! 1G026lv1G031l\"=l:merged_text_edit[''merged''][''newText'']\rp'"] Wed 26 Jun 2019 10:56:22 AM CEST:["s:on_text_document_did_close()", 1] Wed 26 Jun 2019 10:56:23 AM CEST:["s:on_text_document_did_open()", 2, "tex", "/scratch/test", "file:///scratch/test/test1.tex"] Wed 26 Jun 2019 10:56:23 AM CEST:[{"response": {"data": {"data": "vim-lsp", "server_name": "texlab"}, "message": "server already started"}}] Wed 26 Jun 2019 10:56:23 AM CEST:[{"response": {"data": {"data": "vim-lsp", "init_result": {"id": 1, "jsonrpc": "2.0", "result": {"capabilities": {"documentHighlightProvider": true, "hoverProvider": true, "documentLinkProvider": {"resolveProvider": false}, "documentFormattingProvider": true, "foldingRangeProvider": true, "textDocumentSync": {"save": {"includeText": false}, "change": 1, "openClose": true}, "renameProvider": true, "definitionProvider": true, "completionProvider": {"resolveProvider": true, "triggerCharacters": ["\", "{", "}", "@", "/", " "]}, "referencesProvider": true}}}, "server_name": "texlab"}, "message": "lsp server already initialized"}}] Wed 26 Jun 2019 10:56:23 AM CEST:[{"response": {"data": {"data": "vim-lsp", "server_name": "texlab"}, "message": "configuration sent"}}] Wed 26 Jun 2019 10:56:23 AM CEST:["s:update_file_content()", 2] Wed 26 Jun 2019 10:56:23 AM CEST:["--->", 5, "texlab", {"method": "textDocument/didOpen", "params": {"textDocument": {"uri": "file:///scratch/test/test1.tex", "version": 1, "languageId": "tex", "text": "\section{Section 1}\label{sec:1}\n"}}}] Wed 26 Jun 2019 10:56:23 AM CEST:[{"response": {"data": {"path": "file:///scratch/test/test1.tex", "data": "vim-lsp", "filetype": "tex", "server_name": "texlab"}, "message": "textDocument/open sent"}}] Wed 26 Jun 2019 10:56:23 AM CEST:[{"response": {"data": {"path": "file:///scratch/test/test1.tex", "data": "vim-lsp", "server_name": "texlab"}, "message": "not dirty"}}] Wed 26 Jun 2019 10:56:23 AM CEST:["s:build_cmd", "keepjumps keepalt b 1 | execute 'keepjumps normal! 3G041lv3G046l\"=l:merged_text_edit[''merged''][''newText'']\rp'"] Wed 26 Jun 2019 10:56:23 AM CEST:["s:on_stdout client request on_notification() error", "Vim(buffer):E37: No write since last change (add ! to override)", "function 137_on_stdout[4]..136_on_stdout[79]..44[1]..145_handle_workspace_edit[10]..lsp#utils#workspace_edit#apply_workspace_edit[15]..lsp#utils#text_edit#apply_text_edits, line 72"] Wed 26 Jun 2019 10:56:23 AM CEST:["<---", 5, "texlab", {"response": {"method": "textDocument/publishDiagnostics", "jsonrpc": "2.0", "params": {"diagnostics": [], "uri": "file:///scratch/test/test2.tex"}}}] Wed 26 Jun 2019 10:56:23 AM CEST:["<---", 5, "texlab", {"response": {"method": "textDocument/publishDiagnostics", "jsonrpc": "2.0", "params": {"diagnostics": [], "uri": "file:///scratch/test/test3.tex"}}}] Wed 26 Jun 2019 10:56:23 AM CEST:["s:on_stdout on_notification() error", "Vim(call):E5555: API call: Wrong type for argument 1, expecting Buffer"] Wed 26 Jun 2019 10:56:23 AM CEST:["<---", 5, "texlab", {"response": {"method": "textDocument/publishDiagnostics", "jsonrpc": "2.0", "params": {"diagnostics": [], "uri": "file:///scratch/test/test.tex"}}}] Wed 26 Jun 2019 10:56:23 AM CEST:["s:on_stdout on_notification() error", "Vim(call):E5555: API call: Wrong type for argument 1, expecting Buffer"] Wed 26 Jun 2019 10:56:23 AM CEST:["<---", 5, "texlab", {"response": {"method": "textDocument/publishDiagnostics", "jsonrpc": "2.0", "params": {"diagnostics": [], "uri": "file:///scratch/test/test1.tex"}}}] Wed 26 Jun 2019 10:56:23 AM CEST:[{"response": {"data": {"data": "vim-lsp", "server_name": "texlab"}, "message": "server already started"}}] Wed 26 Jun 2019 10:56:23 AM CEST:[{"response": {"data": {"data": "vim-lsp", "init_result": {"id": 1, "jsonrpc": "2.0", "result": {"capabilities": {"documentHighlightProvider": true, "hoverProvider": true, "documentLinkProvider": {"resolveProvider": false}, "documentFormattingProvider": true, "foldingRangeProvider": true, "textDocumentSync": {"save": {"includeText": false}, "change": 1, "openClose": true}, "renameProvider": true, "definitionProvider": true, "completionProvider": {"resolveProvider": true, "triggerCharacters": ["\", "{", "}", "@", "/", " "]}, "referencesProvider": true}}}, "server_name": "texlab"}, "message": "lsp server already initialized"}}] Wed 26 Jun 2019 10:56:23 AM CEST:[{"response": {"data": {"data": "vim-lsp", "server_name": "texlab"}, "message": "configuration sent"}}] Wed 26 Jun 2019 10:56:23 AM CEST:[{"response": {"data": {"path": "file:///scratch/test/test1.tex", "data": "vim-lsp", "server_name": "texlab"}, "message": "already opened"}}] Wed 26 Jun 2019 10:56:23 AM CEST:["s:update_file_content()", 2] Wed 26 Jun 2019 10:56:23 AM CEST:["--->", 5, "texlab", {"method": "textDocument/didChange", "params": {"contentChanges": [{"text": "\section{Section 1}\label{sec:one}\n"}], "textDocument": {"uri": "file:///scratch/test/test1.tex", "version": 2}}}] Wed 26 Jun 2019 10:56:23 AM CEST:[{"response": {"data": {"path": "file:///scratch/test/test1.tex", "data": "vim-lsp", "server_name": "texlab"}, "message": "textDocument/didChange sent"}}] Wed 26 Jun 2019 10:56:23 AM CEST:["--->", 5, "texlab", {"method": "textDocument/documentHighlight", "on_notification": "---funcref---", "params": {"textDocument": {"uri": "file:///scratch/test/test1.tex"}, "position": {"character": 33, "line": 0}}}] Wed 26 Jun 2019 10:56:23 AM CEST:["s:on_text_document_did_change()", 2] Wed 26 Jun 2019 10:56:23 AM CEST:["s:send_didchange_queue() will be triggered"] Wed 26 Jun 2019 10:56:23 AM CEST:["<---", 5, "texlab", {"response": {"method": "textDocument/publishDiagnostics", "jsonrpc": "2.0", "params": {"diagnostics": [], "uri": "file:///scratch/test/test2.tex"}}}] Wed 26 Jun 2019 10:56:23 AM CEST:["<---", 5, "texlab", {"response": {"method": "textDocument/publishDiagnostics", "jsonrpc": "2.0", "params": {"diagnostics": [], "uri": "file:///scratch/test/test3.tex"}}}] Wed 26 Jun 2019 10:56:23 AM CEST:["s:on_stdout on_notification() error", "Vim(call):E5555: API call: Wrong type for argument 1, expecting Buffer"] Wed 26 Jun 2019 10:56:23 AM CEST:["<---", 5, "texlab", {"response": {"method": "textDocument/publishDiagnostics", "jsonrpc": "2.0", "params": {"diagnostics": [], "uri": "file:///scratch/test/test.tex"}}}] Wed 26 Jun 2019 10:56:23 AM CEST:["s:on_stdout on_notification() error", "Vim(call):E5555: API call: Wrong type for argument 1, expecting Buffer"] Wed 26 Jun 2019 10:56:23 AM CEST:["<---", 5, "texlab", {"response": {"method": "textDocument/publishDiagnostics", "jsonrpc": "2.0", "params": {"diagnostics": [], "uri": "file:///scratch/test/test1.tex"}}}] Wed 26 Jun 2019 10:56:23 AM CEST:["<---", 5, "texlab", {"response": {"id": 7, "jsonrpc": "2.0", "result": [{"range": {"end": {"character": 33, "line": 0}, "start": {"character": 26, "line": 0}}, "kind": 3}]}, "request": {"id": 7, "jsonrpc": "2.0", "method": "textDocument/documentHighlight", "params": {"textDocument": {"uri": "file:///scratch/test/test1.tex"}, "position": {"character": 33, "line": 0}}}}] Wed 26 Jun 2019 10:56:24 AM CEST:["s:on_text_document_did_close()", 2] Wed 26 Jun 2019 10:56:24 AM CEST:["s:on_exit", 5, "texlab", "exited", 0]


2. Failing case:

Wed 26 Jun 2019 10:56:05 AM CEST:["--->", 5, "texlab", {"method": "textDocument/rename", "on_notification": "---funcref---", "params": {"newName": "sec:one", "textDocument": {"uri": "file:///scratch/test/test1.tex"}, "position": {"character": 30, "line": 0}}}] Wed 26 Jun 2019 10:56:05 AM CEST:["<---", 5, "texlab", {"response": {"id": 4, "jsonrpc": "2.0", "result": {"changes": {"file:///scratch/test/test.tex": [], "file:///scratch/test/test1.tex": [{"range": {"end": {"character": 31, "line": 0}, "start": {"character": 26, "line": 0}}, "newText": "sec:one"}], "file:///scratch/test/test2.tex": [{"range": {"end": {"character": 46, "line": 2}, "start": {"character": 41, "line": 2}}, "newText": "sec:one"}], "file:///scratch/test/test3.tex": []}}}, "request": {"id": 4, "jsonrpc": "2.0", "method": "textDocument/rename", "params": {"newName": "sec:one", "textDocument": {"uri": "file:///scratch/test/test1.tex"}, "position": {"character": 30, "line": 0}}}}] Wed 26 Jun 2019 10:56:05 AM CEST:["s:build_cmd", "keepjumps keepalt b 1 | execute 'keepjumps normal! 1G026lv1G031l\"=l:merged_text_edit[''merged''][''newText'']\rp'"] Wed 26 Jun 2019 10:56:05 AM CEST:["s:build_cmd", "keepjumps keepalt edit /scratch/test/test2.tex | execute 'keepjumps normal! 3G041lv3G046l\"=l:merged_text_edit[''merged''][''newText'']\rp'"] Wed 26 Jun 2019 10:56:05 AM CEST:["s:on_stdout client request on_notification() error", "Vim(edit):E37: No write since last change (add ! to override)", "function 137_on_stdout[4]..136_on_stdout[79]..26[1]..145_handle_workspace_edit[10]..lsp#utils#workspace_edit#apply_workspace_edit[15]..lsp#utils#text_edit#apply_text_edits, line 72"] Wed 26 Jun 2019 10:56:05 AM CEST:["s:on_text_document_did_change()", 1] Wed 26 Jun 2019 10:56:05 AM CEST:["s:send_didchange_queue() will be triggered"] Wed 26 Jun 2019 10:56:08 AM CEST:["s:on_text_document_did_close()", 1] Wed 26 Jun 2019 10:56:08 AM CEST:["s:on_exit", 5, "texlab", "exited", 0]



So it seems that it's actually finding all instances correctly in both cases, but the rename action isn't applied in the other files in the failing case. 
thomasfaingnaert commented 5 years ago

@clason I believe this is because you have set nohidden (which is the default). When Vim switches buffers to apply renames, it abandons the current buffer. If the current buffer is modified, and hidden is not set, Vim will abort the rename. You may want to read up on :h 'hidden', :h abandon and :h E37. Personally, I have had set hidden in my vimrc for a long time, and have not had any problems. You cannot quit Vim when there are hidden buffers with modifications, unless you do :qall!. Could you check if set hidden in your vimrc solves this? I'll check tomorrow if maybe we can fix this in vim-lsp as well.

Unrelated: I've been using lervag/vimtex for some time, and I noticed you switched to a language server for LaTeX. I was wondering if there are any features you were missing in vimtex?

clason commented 5 years ago

@thomasfaingnaert Indeed, set hidden fixes this. I was aware of this option in the context of language servers, but didn't think of it here because that should have also applied to the first, working, example. (If I may ask: for the working example, where the new buffer is actually shown, a single undo undoes the rename in both files; with set hidden, it only undoes the rename in the current buffer. I find the first behavior convenient; is there some option I can set to get it with set hidden as well?)

And yes, vimtex is excellent, and I have no intention of switching. Someone requested LSP integration in an issue I saw, and I got curious (read: procrastinated since it was way too hot for doing mathematics; I also saw the recent work in neovim for providing explicit hooks for language client -- I'd be interested in how this impacts vim-lsp). Vimtex provides a lot of improvements on basic vim functionality for TeX files (highlighting, indentation, text objects -- cse and tse/d are game-changers) which do not make sense in a language server; the continuous compilation etc. are also very convenient and not (yet?) provided by texlab (although the VS Code extension around it does). On the other hand, I do find the project-wide rename option for labels and BibTeX keys convenient (for labels, :bufdo %s/{A}/{B}/g | update does the trick as well, but BibTeX keys are more difficult), and "Go to Definition" on labels and keys (jumps -- or previews with Peek to the \label or @... from a \ref or \cite) as well as "List all references" (puts all \*ref or \cite for a given \label or @... in the quickfix list) could be very useful. (Especially since they don't require aux files, i.e., having run latex at least once, to work.) So I see them as essentially complementary.

The only real overlap seems to be completion; here vimtex sets a very high bar with regard to features (a lot of work has gone into this, after all): context-aware completion (\eqref{ without any other label part only shows equations, \end{ always shows the current environment as first candidate), fuzzy matching (labels match formatted labels, i.e., \eqref{2.1<c-x c-o> completes to \eqref{eq1} if that equation gets the tag (2.1), or BibTeX keys match parts of author or title strings), etc. It also auto-inserts closing braces when completing, e.g., environments (is this something vim-lsp can do as well?) On the other hand, it can be quite slow in large projects, and requires aux files, and here the language server pulls ahead. I don't know yet where my personal tipping point is where I'd choose speed over features. (Although in principle it's possible to have both, I think, at least for ncm2-based autocomplete: if the omnicomplete source has higher priority, I could get the fast completions first, and if there is already an aux file and vimtex provides the "better" completion, they show over the language server ones when they come in.)

thomasfaingnaert commented 5 years ago

@clason Are you sure that the rename is applied to both files in the first case? I think it should only be changed in test1.tex, and not in test2.tex.

u would then undo each change individually in both cases. We might want to group text edits for LspRename with :undojoin though.

EDIT: Actually, I don't think that will work across buffers.

clason commented 5 years ago

@thomasfaingnaert Yes, the rename is applied to both files (with set hidden); this is the desired behavior. (It's a refactoring action for when you decide that while {eq1} may have been OK when you were drafting the first chapter, now that you're in Chapter 4 you start regretting your life decisions.)

That the renames have to be undone per buffer is less convenient than the alternative but hardly a deal-breaker. (Weird that it did behave this way in the working case with set nohidden; some parts of vim are still a mystery to me.)

thomasfaingnaert commented 5 years ago

@clason I meant with set nohidden: it would be strange if example 1 worked, but 2 didn't. If so, something else may be going on.

clason commented 5 years ago

@thomasfaingnaert Yes, as I wrote: it's weird. I can try to produce additional logs, if that would help?

thomasfaingnaert commented 5 years ago

@clason So I tried this myself, and I got this: Peek 2019-06-27 22-56

The rename is applied to test1.tex, but not test2.tex, as expected.

clason commented 5 years ago

@thomasfaingnaert Hmm, good call. You're right. I could have sworn that it updated both files earlier. So the behavior is different for labels and references: It always starts with the jumping to the definition, renaming that, and then either stops (if it's an open buffer with nohidden) or proceeds to the references, probably in occurrence order (if it's a hidden buffer). That makes sense.

🤦‍♂

I do appreciate your help; sorry to have spammed so many non-issues in such a short time...

thomasfaingnaert commented 5 years ago

@clason I think it probably applies the edits in alphabetical order of filename, not that it matters of course :p

clason commented 5 years ago

Also makes sense. I notice from your neat ascii-cinema that your LspRename action offers as "new name "sec:1" -- with the colon. But I only get the first or second part (depending on which side of the colon the cursor is) -- another spurious issue I opened. Can you guess what the difference might be?

thomasfaingnaert commented 5 years ago

My guess is that it's because : is not in your 'iskeyword'?

clason commented 5 years ago

Ah, OK. I really need to find out what part of my setup removes that (it's set by vimtex). That's a different issue, though.

(Manually setting iskeyword+=: fixes it, so you're right.)

thomasfaingnaert commented 5 years ago

@clason :verbose set iskeyword? might help

clason commented 5 years ago

That's a good tip, I'll try that!