Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Line alignment makes hard to understand Extract Variable refactoring #120

Open
tsantalis opened this issue Mar 9, 2023 · 5 comments
Open
Assignees

Comments

@tsantalis
Copy link

jodavimehran/code-tracker@daeaccb

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
Copy link
Author

@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
Copy link
Member

onewhl commented Mar 14, 2023

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

@onewhl onewhl assigned onewhl and unassigned anchouls Mar 24, 2023
@onewhl
Copy link
Member

onewhl commented Mar 24, 2023

@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
Copy link
Author

tsantalis commented Mar 25, 2023

@onewhl @anchouls
I think my comment was misunderstood.

The highlighed lines in #120 (comment) 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
Copy link
Member

onewhl commented Mar 27, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants