search

JetBrains-Research / RefactorInsight

An IntelliJ IDEA plugin that detects refactorings in Git commits
https://plugins.jetbrains.com/plugin/14704-refactorinsight
MIT License
99 stars 9 forks source link

Line alignment makes hard to understand Extract Variable refactoring #120

Open tsantalis opened 1 year ago

tsantalis commented 1 year ago

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 commented 1 year ago

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

onewhl commented 1 year ago

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

onewhl commented 1 year ago

@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
tsantalis commented 1 year ago

@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.

onewhl commented 1 year ago

@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.