latex-lsp / texlab

An implementation of the Language Server Protocol for LaTeX
GNU General Public License v3.0
1.48k stars 51 forks source link

Completions with no closing brace too greedy #559

Closed tgray closed 2 years ago

tgray commented 2 years ago

Using texlab (installed with homebrew on macOS) with BBEdit 14.1 for completions. Sometimes asking for a completion of a tex command where the insertion point is after a left brace (\begin{ or \cite{), the return text range is nmuch too large and ends up replacing too much text, often to the next comma, slash, or brace. For example, cursor at the |:

\begin{verb|

Velit tri-tip fig1n shoulder buffalo pariatur porkchop magna chuck sausage,
sed hamburger fatback ribeye biltong id lorem culpa cow, frankfurter
deserunt shortloin pancetta dolor et veniam aliqua andouille, pork fugiat eu
pig landjaeger proident aliquip voluptate. 

Trying to complete 'verbatim' will result in:

\begin{verbatim, sed hamburger fatback ribeye biltong id lorem culpa cow,
frankfurter deserunt shortloin pancetta dolor et veniam aliqua andouille,
pork fugiat eu pig landjaeger proident aliquip voluptate. 

Verbose logging shows that the replacement range spans lines:

2022-02-09 07:13:11.895: stderr output from server: DEBUG - <
{"jsonrpc":"2.0","method":"textDocument\/didChange","params":{"
contentChanges":[{"range":{"start":{"line":49,"character":7},"end":{"line":
51,"character":74}},"text":"verbatim"}],"textDocument":{"uri":"file:\/\/\/
Users\/tgray\/Documents\/Misc\/2022\/latex-test\/latex-test.tex","version":
152}}}

Does texlab only provide completion suggestions with the assumption that there is a matching closing brace already? In other words \begin{verb|} (cursor at the |).

clason commented 2 years ago

I think this is related to some earlier issues (like https://github.com/latex-lsp/texlab/issues/510); I see a similar problem in Neovim (where the replacement is correct, but the server crashes afterwards).

pfoerster commented 2 years ago

Does texlab only provide completion suggestions with the assumption that there is a matching closing brace already? In other words \begin{verb|} (cursor at the |).

Thanks for the report. This problem is not necessarily unrelated to the missing closing brace. Instead, the problem lies in the ambiguity of LaTeX. If you write \ref{foo bar} for example, then this command references \label{foo bar}. Most identifiers are allowed to contain spaces (xkeyval is another example) so the completion tries to replace the whole identifier. Obviously, this heuristic fails here.

As a solution, we could do one of the following:

  1. Never replace multiple lines by restricting the range in the completion (this is now required by the LSP spec for completion items)
  2. Update the parser to disallow identifiers with line breaks (probably the right way although the compatibility with tree-sitter needs to be checked). This also fixes the LSP spec violation.
tgray commented 2 years ago

Your response makes a lot of sense. This one is relatively easy to work around with balanced braces for now, but I bet 99% of use cases without the trailing brace would be fixed by #1. Glad to hear the report was useful.

pfoerster commented 2 years ago

Can you try out 4dd2518, please? The issue should be fixed now.

tgray commented 2 years ago

Seems better. When \cite{ is on its own line, the completion doesn't go past the line breaks. Thanks!

pfoerster commented 2 years ago

Released with texlab 3.3.2.