Initial work towards prefixes for custom label commands.

[DasNaCl]

This commit mainly extends the configs with a map from label command name to the respective prefix, as outlined in #1138. It does not yet implement any functionality, since it appears that base-db/semantics/label needs an extension carrying a span that contains the label command. The plan is to extend the logic in crates/diagnostics/src/labels.rs.

For discussion on this, as outlined in CONTRIBUTING.md, please refer to the accompanying issue.

[pfoerster]

Thanks for working on this!

I propose to add another option to the server that allows a mapping from custom label commands to a prefix, which would be considered when checking for duplicate or unused references.

Sure, this sounds reasonable. Adding this would also help supporting the xr package (#873), where you can import the references from another file while appending a prefix.

because base_db::semantics::tex::Label or ...::LabelKind do not appear to carry information about the actual command.

I think there are 2 ways to implement this:

1. Modify the lexer token to include the prefix in CommandName::LabelDefinition and propagate that through the parser including the SyntaxKind enum needed for rowan. This would be quite an invasive change as you mentioned.
2. Map the label command name to a prefix at during the semantics pass. Here we still have the original command name when constructing the Label struct. Probably, it would be easiest to use the name including the prefix as the actual name for the label. Then, something like this would also work without additional changes:

\newcommand{\asm}[2]{\item\label[asm]{asm:#1} {#2}}
\newcommand{\asmref}[1]{\Cref{asm:#1}}

% semantics module will store a label `asm:foo` internally
\asm{foo} 

% Goto definition return `\asm{foo}`
\ref{asm:foo}

One notable change would be that Span::text::len() would no longer be equal to the length of Span::range necessarily but I don't think that this would break anything.

Special care would be needed completing commands like \asmref because these need to strip the prefix of the command from the returned label list (but this would also help us to do the filtering here, since we can omit labels that do not start with asm: from the completion results).

[DasNaCl]

I decided to go with the second option, but now that I want to get completion up to speed, it looks like the first option may be necessary. The issue is that, without Label carrying the corresponding, defining command, I can't seem to find a way to identify the command used to define a label. This is necessary in order to prepend the prefix (see the currently failing test, trying to reference a label using \ref instead of the custom reference command: https://github.com/DasNaCl/texlab/blob/1f2eb76c8fb0b268d57a7ea9769532210fc20daa/crates/completion/src/tests.rs#L2170).

@pfoerster Maybe you have another idea? :-)

[DasNaCl]

@pfoerster Thank you for the quick look! Indeed, this was one of many other typos I identified in the added tests. I've shrunken the tests a bit down and removed some unnecessary TeX around it.

Here is a minimal working example of a document using custom definition and reference commands with custom prefixes:

\documentclass{article}
\newcommand{\asm}[2]{\item\label[asm]{asm:#1} {#2}}
\newcommand{\asmref}[1]{\ref{asm:#1}}
\newcommand{\goal}[2]{\item\label[goal]{goal:#1} {#2}}
\newcommand{\goalref}[1]{\ref{goal:#1}}
\begin{document}
    \begin{enumerate}\label{baz}
        \asm{foo}{what}
        \goal{bar}{what}
    \end{enumerate}

    \asmref{}  % suggests "foo"
    \goalref{} % suggests "bar"
    \ref{}     % suggests "baz", "asm:foo", and "goal:bar"
\end{document}

To make this work in my editor, I've configured texlab with:

      experimental = {
        labelReferenceCommands = {"asmref","goalref"},
        labelDefinitionCommands = {"asm","goal"},
        labelReferencePrefixes = {{"asmref", "asm:"}, {"goalref", "goal:"}},
        labelDefinitionPrefixes = {{"asm", "asm:"}, {"goal", "goal:"}}
      },

The pull request seems to be feature complete now, for what I can tell. :)

[DasNaCl]

Thanks @DasNaCl, the completion works nicely!

While testing, I found one issue with the rename request.

In your example, if you rename \ref{asm:foo} to asm:bar. Then, \asmref{foo} will be renamed to \asmref{asm:bar}.

Haaa, of course, makes sense that stuff like this breaks. Forgot about that! I just checked other functions, like getting a list of all references or going to a definition, but those appear to work even with just \ref. After looking at how renaming works, it looks like this is way more tricky to achieve, since only text positions are recorded and then naively replaced one-by-one with the respective new name. But, it is necessary to know the kinds of commands associated to the labels (which we don't have https://github.com/DasNaCl/texlab/blob/7243f37678e58ad46ffb1504edc482064cf7edce/crates/rename/src/label.rs#L19 :upside_down_face:) which, again, suggests the earlier discussed, more invasive change of remembering the label-definition or label-reference command within Label itself. Let me know if you have another idea!

Adding this would also help supporting the xr package (https://github.com/latex-lsp/texlab/issues/873), where you can import the references from another file while appending a prefix.

I looked into this today and saw that this also necessitates an extension of another "includes"-kind, similar to how subfiles work. Without that, texlab fails to, e.g., suggest completions or spuriously reports references as undefined.

[pfoerster]

What gets lost is the ability to map one command to multiple different prefixes, which may be useful if someone \renewcommands.

Well, in this case, the list of tuples is better suited so I suggest we stick with tuples even if we just support one prefix for now.

After looking at how renaming works, it looks like this is way more tricky to achieve, since only text positions are recorded and then naively replaced one-by-one with the respective new name.

Previously, the rename code used to record a Vec<Span> for each document with the required changes. If we change it back to this way, then we can use a different text depending on the prefix.

remembering the label-definition or label-reference command within Label itself.

Probably, there is no way around extending Label with a prefixes: Vec<String> property and modifying the places where equality checks are done with the name.

[DasNaCl]

0d065c9 realizes the desired feature. I've also added some tests for renaming with prefixes. (not sure how valuable they are, though, since it doesn't seem they check for what is actually being replaced, but just matching the text positions)

In some hand-made tests on some small and big TeX files in my personal collection, the feature appears to work for any combination of \ref, \asmref, \label, or \asm. I managed to implement it without going back to having a Vec<Span>. Instead, the existing Vec<TextRange> was modified to be a Vec<RenameInformation>. RenameInformation is a new struct that contains the TextRange and, maybe, a prefix. The method of implementing it this way has the slight drawback of polluting the API for the other renaming operations, e.g., commands or citations. Nevertheless, I've found it simpler than rolling back to Vec<Span>. Label is extended with an Option<String> representing a prefix that needs to be prepended to new_name.

@pfoerster Let me know if you identify additional issues and thank you for your ongoing support.

Well, in this case, the list of tuples is better suited so I suggest we stick with tuples even if we just support one prefix for now.

I rolled back now in 3b4d1d0 to the Vec<(String,String)> representation and also replaced the other FxHashMap representations for prefixes by Vec. However, I did not change the code to account for such \renewcommands and I doubt those would happen often in practice anyway. I believe the costs of implementing such a feature right now outweigh the benefits of merging this PR before having support for these redefs, e.g., renaming would need more complicated logic (keeping track of a command re-definition and using the corresponding element in the Vec accordingly, i.e., before redefinition, use the first found alternative, after redefinition, skip the first and use the next, after another redefinition, skip the second as well and...]).

[pfoerster]

0d065c9 realizes the desired feature.

Thank you for the update!

@pfoerster Let me know if you identify additional issues and thank you for your ongoing support.

While testing, I found one remaining issue (although this is a rare case):

When renaming, asm:foo, then \asmref{foo} has an additional asm:-prefix.

The rest works just fine :)

[DasNaCl]

@pfoerster sorry, I cannot reproduce and from what I can reproduce, I see no issues... See an attempt in action:

The way it's implemented is that it 'intelligently' identifies whether a label has an associated prefix and prepends it accordingly when doing the renaming. -> There is no need to write asm:bar, just bar is enough and texlab replaces the relevant places. Renaming with asm:bar consistently rewrites all locations, so \ref will have to asm: prefixes and \asmref as well as \asm just one.

Thank you again for taking time to look at this!

[pfoerster]

There is no need to write asm:bar, just bar is enough and texlab replaces the relevant places.

Yeah, allowing the user to write asm:bar would introduce another issue, where you omit the asm: part but renaming in this case is not possible.

Thank you again for taking time to look at this!

Thank you for contributing!