Environment missing label inspection

[fberlakovich]

Fixes #874

Summary of additions and changes

• Implements the missing label inspection + fix for environments (including figures), except for lstlistings
• Change the existing quickfix to modify the PSI tree instead of text insertion. Unfortunately this automatically applies code formatting. If this is unwanted, I can look into ways of preventing it.
• Fix the getName implementation of LatexCommands to use the stub if available and the commandName otherwise
• Fix the getOptionalParameters implementation of LatexCommands to adhere to the javadoc

Lstlistings are a special case because they have the label specified as an option. I will fix lstlistings in a separate pull request because this one is already quite big.

How to test this pull request

• Add a figure without a label and apply the suggested quickfix
• With the included testcases

Wiki

• Updated the wiki: https://github.com/Hannah-Sten/TeXiFy-IDEA/wiki/Conventions#Missing-labels

[PHPirates]

Awesome! Thanks for improving the code quality so much :)

Maybe it's personal preference, but do you think it would be nice to insert a newline before the label? I tend to place things like \includegraphics, \caption, \label below each other and not after each other.

[PHPirates]

I'm not entirely clear on why you changed this. Is there any reason why we wouldn't also want the value?

For example, this will fail now:

\documentclass{article}
\usepackage[xindy]{imakeidx}
\makeindex[name=myindex]
\begin{document}
    Some random\index{random} text\index[myindex]{text} which should be indexed\index{index}.
    \printindex[myindex]
\end{document}

because we get optional parameters of the \makeindex command, and we do need to know the name of the index to copy.

[fberlakovich]

Is there any reason why we wouldn't also want the value?

The reason why I changed it was because the javadoc states it should return only the names and from what I could tell no client made use of the parameter values (please correct me if I am wrong). It would be nice to have a method which also returns the values (e.g. as a Map), but this method would also have to take care of parameters like \makeindex[name={myindex}] which the previous implementation did not. But I can definitely extend the current method if needed.

because we get optional parameters of the \makeindex command, and we do need to know the name of the index to copy.

Could you explain the example in more detail please? What do you mean with "know the name of the index to copy"?

[PHPirates]

Ah, well I could only find one client which made use of it: the makeindex run configuration, which does the splitting on = itself to create a map of it (MakeindexRunConfiguration:162). Would probably be better to put that in a separate method indeed, and leave the one you fixed as it now is.

In that example, imakeidx generates myindex.idx, on which we run texindy, and then copy the generated myindex.ind back next to the main file, because imakeidx is not so smart and can't find the .ind file otherwise. It's not so important how, but I believe we do need the value of the optional parameters of \makeindex there.

[fberlakovich]

Ah, well I could only find one client which made use of it: the makeindex run configuration, which does the splitting on = itself to create a map of it (MakeindexRunConfiguration:162). Would probably be better to put that in a separate method indeed, and leave the one you fixed as it now is.

I must have missed it then. I will add a version that also returns the parameter values.

[fberlakovich]

@PHPirates I updated the implementation of getOptionalParameters now to create a map of parameters and values and also updated the existing clients to use the maps keyset where applicable.

[fberlakovich]

Maybe it's personal preference, but do you think it would be nice to insert a newline before the label? I tend to place things like \includegraphics, \caption, \label below each other and not after each other.

Yes, I agree, but according to https://www.jetbrains.org/intellij/sdk/docs/basics/architectural_overview/modifying_psi.html#whitespaces-and-imports whitespaces should not be created through PSI modifications. We should rather adapt the formatter which might also address #1179

[PHPirates]

Great! As far as I can see it works correctly now.