RefactorInsight icon indicating copy to clipboard operation
RefactorInsight copied to clipboard

Line alignment makes hard to understand Extract Variable refactoring

Open tsantalis opened this issue 1 year ago • 5 comments

https://github.com/jodavimehran/code-tracker/commit/daeaccb67f2f1510ae15e45727ed1347d9368bdb

Lines R431 and R432 are new and correspond to the extracted variables fragment1 and fragment2 It would help to align the code as follows to make the visualization more clear

L428 -- R430 L429 -- R433 L433 -- R437

Screenshot from 2023-03-09 12-46-19

Overall, the diff does not show properly the added code, shown in green below

Screenshot from 2023-03-09 13-06-47

tsantalis avatar Mar 09 '23 18:03 tsantalis

@onewhl Idea for improving the visualization: You can use the statement mappings of the parent method isMatched() to get the statements that are added.

tsantalis avatar Mar 09 '23 18:03 tsantalis

@anchouls please investigate if it's possible to highlight all occurrences of extracted variable "fragment1" in the right part of the diff.

onewhl avatar Mar 14 '23 13:03 onewhl

@tsantalis @anchouls I tried to highlight more lines, but I didn't find a way to get all occurrences of a new variable in the method. @tsantalis what do you do to get information about lines 435 and 439?

Here is the visualization of Extract Variable fragment1: Screen Shot 2023-03-24 at 3 17 10 PM

onewhl avatar Mar 24 '23 14:03 onewhl

@onewhl @anchouls I think my comment was misunderstood.

The highlighed lines in https://github.com/JetBrains-Research/RefactorInsight/issues/120#issue-1617746136 are correct. There is no need to highlight more things.

The problem is that the diff does not show which lines are deleted and added. This is what creates the confusion.

Lines R427-428, R439-440 and R448-455 are added.

My understanding is that you get the raw text from the left and right, and you just overlay the information related to the refactoring, without doing any textual diff of the code.

If you get the UMLOperationBodyMapper for the parent method containing the refactoring, you can get all added statements by calling getNonMappedInnerNodesT2() and getNonMappedLeavesT2() you can get all deleted statements by calling getNonMappedInnerNodesT1() and getNonMappedLeavesT1()

These statements can be highlighted with background colours, so that it's clear what is added and deleted code.

This process can be applied for all kinds of refactorings, visualized within a method body.

tsantalis avatar Mar 25 '23 20:03 tsantalis

@tsantalis the idea of this mode is to visualize refactoring in isolation, so only refactoring-related lines should be highlighted. That's why we don't highlight lines that don't relate to the refactoring.

I agree that it would be much better if we highlight added lines with green color, deleted with gray, and changed with blue. Currently, it works not for all refactoring types but we're working on it.

Regular diff, that IDE builds, highlights all changes with different colors. And plugin integrated information about refactorings in regular diff via toggles and inlay hints.

onewhl avatar Mar 27 '23 11:03 onewhl