Extract value refactoring

[jojo2357]

Fix #3243

Currently trying to implement at minimum a basic 'extract duplicated code' feature (not the inspection)

TODO:

• Investigate math
• make findOccurrences visit all inclusions in this file, and to go upstream via inclusions to this file
• find sticking points
• with no selection, offer current text word, current text line, current paragraph, maybe more, but most importantly offer current sentence (line)
• Clean the code (like, a LOT)

Now I really want introduceParameter for a custom command. Would that be extremely hard?

[PHPirates]

This looks very promising!

• I really like how you reused OccurrencesChooser
• I would put the \newcommand simply in the preamble after the package inclusions, just like how we do it for inserting a \usepackage command, insertUsepackage()
• Don't forget to comment your code while you're writing it! Even if it's just rough code that doesn't work yet. it's much easier to do immediately than trying to remember afterwards what it's doing and why.
• Indeed there are some edge cases to do, but the general structure of the code is good I think. It's good that you asked for a quick review already.

[jojo2357]

I do not know how to navigate the PSI tree clearly.

When attempting to refactor on

\documentclass[11pt]{article}

\begin{document}

    \chapter{The significance of 2.718, or e}

    2.718 is a special number.

    $\lim_{x \to \infty} (1+\frac{1}{x})^{x}=2.718$

    Some old guy discovered 2.7<carret>18 and was amazed!

    Not to be confused with 2.714, or  $3\pi!-6\pi$

\end{document}

And the following are the parents that I get. Each line is a parent, newlines have been converted to \n

2.718
Some old guy discovered 2.718 and was amazed!\n\n    Not to be confused with 2.714, or
Some old guy discovered 2.718 and was amazed!\n\n    Not to be confused with 2.714, or
\chapter{The significance of 2.718, or e}\n\n    2.718 is a special number.\n\n    $\lim_{x \to \infty} (1+\frac{1}{x})^{x}=2.718$\n\n    Some old guy discovered 2.718 and was amazed!\n\n    Not to be confused with 2.714, or  $3\pi!-6\pi$
\begin{document}\n\n    \chapter{The significance of 2.718, or e}\n\n    2.718 is a special number.\n\n    $\lim_{x \to \infty} (1+\frac{1}{x})^{x}=2.718$\n\n    Some old guy discovered 2.718 and was amazed!\n\n    Not to be confused with 2.714, or  $3\pi!-6\pi$\n\n\end{document}
\begin{document}\n\n    \chapter{The significance of 2.718, or e}\n\n    2.718 is a special number.\n\n    $\lim_{x \to \infty} (1+\frac{1}{x})^{x}=2.718$\n\n    Some old guy discovered 2.718 and was amazed!\n\n    Not to be confused with 2.714, or  $3\pi!-6\pi$\n\n\end{document}
\documentclass[11pt]{article}\n\n\begin{document}\n\n    \chapter{The significance of 2.718, or e}\n\n    2.718 is a special number.\n\n    $\lim_{x \to \infty} (1+\frac{1}{x})^{x}=2.718$\n\n    Some old guy discovered 2.718 and was amazed!\n\n    Not to be confused with 2.714, or  $3\pi!-6\pi$\n\n\end{document}
\documentclass[11pt]{article}\n\n\begin{document}\n\n    \chapter{The significance of 2.718, or e}\n\n    2.718 is a special number.\n\n    $\lim_{x \to \infty} (1+\frac{1}{x})^{x}=2.718$\n\n    Some old guy discovered 2.718 and was amazed!\n\n    Not to be confused with 2.714, or  $3\pi!-6\pi$\n\n\end{document}

I really want a parent that is Some old guy discovered 2.718 and was amazed! but I dont know if we have that ability. I know we are able to detect multiple sentences on one line, so I figure it is possible.

[jojo2357]

I am very confused over why it breaks in this example:

\documentclass[11pt]{article}
\begin{document}
    
    $5.25 * 2.68291 = 450$

    \begin{table}[ht!]
        \begin{tabular}{| r |}
            \hline
            2.68291 \\
            \hline
        \end{tabular}
    \end{table}

    \begin{table}[ht!]
        \begin{tabular}{| r |}
            \hline
            2.68<caret>291          \\
            \hline
        \end{tabular}
    \end{table}

    Some may wonder why 2.68291 is so special.
\end{document}

and why removing the whitespace after the number magically makes it work. Yes it crashes because I throw the error, but that is because the tree is unexpectedly broken.

[jojo2357]

Ok, so I have one test that fails and I have no IDEA (haha so funny) why and another that fails because it doesnt do what I want yet ™️

[jojo2357]

@PHPirates can you disable checks on my draft pr's? They are a draft because they arent ready 🤯

[jojo2357]

I found that quotes are attached to a word token. This is disappointing because I think that if you select something inside quotes then the quotes should stay and the contents removed. See testWithQuotes to see my sadness

Also I fixed an issue regarding whitespace. If the edge of a selection was on whitespace, it would search in the wrong direction for the real boundary of your selection

[PHPirates]

Regarding navigating the psi tree, I don't see the need to have individual sentences. I hope you should be able to go over all leaf psi elements (or only tokens?) and replace them if needed. Remember that replacing of psi elements is done by creating new psi elements from text using the psi helper (not sure if you already were doing this).
Anyway, I can have a look at the failing tests in more detail (thanks for adding tests!)

can you disable checks on my draft pr's?

Good question, I'll have a look.

[jojo2357]

...creating new psi elements...

Like so?
https://github.com/jojo2357/TeXiFy-IDEA/blob/25974f0feaec0d6f9d48b18289fe81d413eac2e0/src/nl/hannahsten/texifyidea/refactoring/myextractfunction/LatexExtractCommandHandler.kt#L109

[PHPirates]

I looked at the quotes test, and not sure what is currently happening but I would think this is what you need to do:

• Find the psi element under the caret, in this case the whole sentence
• Find the actual selected text
• Create the \newcommand
• Take the sentence text, then string replace the text that was selected by the command, then create a psi element from it and replace it

[jojo2357]

Everything is broken by last commit, but it is the first step before we are able to extract something that isnt represented by something in the PSI tree like a text line or the inside of quotes or something else not covered by the lexer.

[jojo2357]

Fixes #3255

[PHPirates]

I think it would be good to clean up the code first, before you decide to add more features. That would also allow me to review functionality, because currently it's just raising a lot of exceptions and I don't know which cases it should work and which cases it shouldn't.

Would it be possible to have like this small popup 'extract value is not supported here' or some other sort of user feedback in the cases where we don't support it yet? That would be really helpful, also in reviewing.

I'm talking about exceptions like these:

java.lang.IllegalStateException: @NotNull method com/intellij/psi/impl/PsiElementBase.notNullChild must not return null
	at com.intellij.psi.impl.PsiElementBase.$$$reportNull$$$0(PsiElementBase.java)
	at com.intellij.psi.impl.PsiElementBase.notNullChild(PsiElementBase.java:282)
	at nl.hannahsten.texifyidea.psi.impl.LatexCommandsImpl.getCommandToken(LatexCommandsImpl.java:44)
	at nl.hannahsten.texifyidea.psi.impl.LatexCommandsImplMixin.getName(LatexCommandsImplMixin.kt:63)
	at nl.hannahsten.texifyidea.reference.CommandDefinitionReference.multiResolve(CommandDefinitionReference.kt:31)
	at nl.hannahsten.texifyidea.psi.impl.LatexCommandsImplMixin.getReferences(LatexCommandsImplMixin.kt:106)
	at com.intellij.psi.impl.SharedPsiElementImplUtil.addReferences(SharedPsiElementImplUtil.java:58)
	at 

[jojo2357]

Can you add tests for where you make the exception cuz I am not getting those, probably because im not trying hard enough admittedly

[PHPirates]

Very nice! Of course there's always more edge cases, but I think the most important cases are covered.
Now I think it's a matter of cleaning up the code, adding comments, etc. so we can merge it!

[jojo2357]

I am mostly done, let me know what else youd like @PHPirates

[PHPirates]

That would be great! No I think that's all, code quality looks good for the rest, nice job