Support lstinputlisting label

[fberlakovich]

Fix #1694

Summary of additions and changes

• Add general support for labels defined in optional parameters of commands
• Enable autocompletion of labels in lstinputlisting
• Update label inspections and quickfixes to account for the new type of label

How to test this pull request

See the included testcases

Wiki

I guess the existing documentation is already general enough to include this extension.

[fberlakovich]

I am not quite happy with the implementation yet because of the need of the many case distinctions in different places. I think there is a potential for a more general framework, but until such a framework emerges this implementation should be better than nothing.

[PHPirates]

I am not quite happy with the implementation yet because of the need of the many case distinctions in different places. I think there is a potential for a more general framework, but until such a framework emerges this implementation should be better than nothing.

If you mean the manual check for Magic.Command.labelAsParameter everywhere, yes that is inconvenient. But not sure if we could do better, if you think about it, 90% of TeXiFy functionality consists of custom behaviour for certain LaTeX commands.

[fberlakovich]

If you mean the manual check for Magic.Command.labelAsParameter everywhere, yes that is inconvenient. But not sure if we could do better, if you think about it, 90% of TeXiFy functionality consists of custom behaviour for certain LaTeX commands.

Yes, that's what I meant. Of course, the case destinction needs to happen somewhere, but it would be preferrable to have it in a central place. Currently the distinction is scattered over several different places. I think LatexRegularCommand already goes in the right direction as it records metadata for each command in a structured way. Additional metadata could be "defines a label with an optional parameter", "defines a label by itself", that is, a lot of the metadata that is currently mapped in the Magic file. Unfortunately I discovered LatexRegularCommand too late for this PR.

[PHPirates]

True, but even if you use the metadata, you still have the case distinction everywhere, except that then it checks the metadata instead of the information in Magic. The advantage of Magic is that everything is together, though I guess if you would use metadata then you still need to describe the possible 'properties' of a command somewhere in a central place, which would amount to a Magic kind of thing but then not yet specifying which commands have a certain property, as the commands do that themselves.
I'm not yet convinced that it's worth changing that.

PS one other thing I forgot until I got an exception: when changing parser/stub related things you need to update the stub version in LatexParserDefinition, I just did that.

[PHPirates]

Find Usages functionality does not yet work. For that, the reference needs to resolve to the lowest level psi element: e.g. lst:inputlisting in \lstinputlisting[label={lst:inputlisting}]{main2.tex}.
However, there's a still a bug: at the moment that is normal text because it's in a group, but it should be ParameterText, which is a psi element created exactly for this purpose (and has the find usages implemented on it).
So this requires some parser changes, for example creating a ParameterGroup containing ParameterText is the first thing I can think of.
Find Usages is a bit tricky, but I think that should work.

[fberlakovich]

I'm not yet convinced that it's worth changing that.

Neither am I, it is more of a vague feeling that there is potential for improvement. I will think about it.

PS one other thing I forgot until I got an exception: when changing parser/stub related things you need to update the stub version in LatexParserDefinition, I just did that.

Thanks!

[fberlakovich]

I think now it should cover everything. Sorry for the massive PR, I didn't expect the issue to be so complicated.

[PHPirates]

Awesome! Will take a look soon. It's not too bad, there's only like 8 files with 30+ lines changed, rest is minor or generated stuff (which unfortunately we still need to commit to the repo: JetBrains/gradle-grammar-kit-plugin#3)

[fberlakovich]

@PHPirates I hope I understood your points correctly and this is roughly what you had in mind. If not, let me know and I will adapt it.

[PHPirates]

Great, thanks for all the effort! It seems to work now, and also the psi tree is making a lot more sense now than it previously was. I will take a look in more detail tomorrow, but I think all the important parts are there correctly now. I hope you still enjoyed going the extra mile for this PR to improve the parser :)

[fberlakovich]

I will take a look in more detail tomorrow, but I think all the important parts are there correctly now. I hope you still enjoyed going the extra mile for this PR to improve the parser :)

Great! I did indeed and I learned a lot about Latex and the parser.

[PHPirates]

@fberlakovich There's two things I still found, not sure if you have time to fix them:

• There's now a parse error on empty value in optional parameters: \command[key=,] but this has use cases (like in listings parameter, overriding a value to be empty)
• An optional parameter list ending with a comma is now a parse error, but I think it's valid LaTeX

[fberlakovich]

@fberlakovich There's two things I still found, not sure if you have time to fix them:

Sure, I will have a look!

[HannahSchellekens]

@fberlakovich we recently went through the project board (still have to update it) and we stumbled upon these issues as well:

• [bug] duplicate labels for lstlisting do not get detected. it would be nice if this could be added to the functionality.
• [enhancement] Add setting for custom label prefixes. not really important, but because you've been working on the labels, it might be something you could add if you'd like.

[fberlakovich]

@fberlakovich we recently went through the project board (still have to update it) and we stumbled upon these issues as well:

Sure, the duplicate label detection shouldn't be a big thing. Regarding the UI, we already discussed possible implementations on Gitter. I definitely have it on my list, but I can't promise a time frame. I don't have experience with settings/UI stuff and I am not sure yet when I will be able to spare the time. That is, I won't be annoyed if somebody else implements it first ;).