Manual assignment of pairs of files
Context: https://github.com/JabRef/jabref/pull/11180/files
There was code moved from src/main/java/org/jabref/gui/frame/JabRefFrame.java to src/main/java/org/jabref/gui/frame/JabRefFrameViewModel.java. E.g., storeLastOpenedFiles was moved there. I want to see an AST diff there.
Maybe, the use case is that methods go from one file n > 1 other files, which seems to be unsupported currently.
For me, it would be OK, to have a list of candidate files where changes are in and that I select one of the file for viewing (and then getting the MonacoDiff)
src/main/java/org/jabref/gui/frame/JabRefFrame.java
- void storeLastOpenedFiles(List<Path> filenames, Path focusedDatabase) {
- if (prefs.getWorkspacePreferences().shouldOpenLastEdited()) {
- // Here we store the names of all current files. If there is no current file, we remove any
- // previously stored filename.
- if (filenames.isEmpty()) {
- prefs.getGuiPreferences().getLastFilesOpened().clear();
- } else {
- prefs.getGuiPreferences().setLastFilesOpened(filenames);
- prefs.getGuiPreferences().setLastFocusedFile(focusedDatabase);
- }
- }
- }
src/main/java/org/jabref/gui/frame/JabRefFrameViewModel.java
+ void storeLastOpenedFiles(List<Path> filenames, Path focusedDatabase) {
+ if (prefs.getWorkspacePreferences().shouldOpenLastEdited()) {
+ // Here we store the names of all current files. If there is no current file, we remove any
+ // previously stored filename.
+ if (filenames.isEmpty()) {
+ prefs.getGuiPreferences().getLastFilesOpened().clear();
+ } else {
+ prefs.getGuiPreferences().setLastFilesOpened(filenames);
+ prefs.getGuiPreferences().setLastFocusedFile(focusedDatabase);
+ }
+ }
+ }
@koppor
storeLastOpenedFiles() is shown as moved to JabRefFrameViewModel in the diff.
@pouryafard75 Maybe, we could have something like a tooltip/popup that shows the diff when you hover over the moved method.
Side comment (not sure if I should open a new issue)
I don't get the semantics of red and green. In the screenshot the red and green text seems to be the same, but one is marked red, the other one green.
Tracing information: The diff was created by issuing following command
docker run -p 6789:6789 -v c:\users\koppor\rmc:/diff tsantalis/refactoringminer diff --url https://github.com/JabRef/jabref/pull/11180
The hover currently shows "Moved to File: X" multiple times:
Please show it only once!
(If you are on it, file should be with lower letter. Maybe even moved to file: to have the text more passive)
I think, what would help me is to have some letters/icons between the line numbers and the text. For instance M for moved to another file.
Side comment: I miss the connecting lines of IntelliJ. It is so hard for me to correlate left and right - even if a click (somehow) relocates the other scroll pane.
Fun fact: IntelliJ does not recognize that diff. Thus, I can only show at other place how IntelliJ displays:
Note the more lighter colors and the better color contrast?
They use gray for deletion (maybe because of colorblind people)?
@koppor @pouryafard75
Indeed, we need to do a lot to improve the UI look and feel. I agree with all your comments.
To summarize all points in the issue:
- [x] Improve the highlighting of comments within moved code https://github.com/tsantalis/RefactoringMiner/issues/703#issuecomment-2051108649
- [ ] Avoid multiple move tool-tips. Possibly replace it with a single tool-tip that shows the moved code diff https://github.com/tsantalis/RefactoringMiner/issues/703#issuecomment-2051128301
- [x] Introduce connecting lines between left and right side https://github.com/tsantalis/RefactoringMiner/issues/703#issuecomment-2051136238
- [x] Use a different colour pallette more appropriate for colorblind people https://github.com/tsantalis/RefactoringMiner/issues/703#issuecomment-2051155616
- [ ] Use a global setting (cookie) to store the preferred diff editor of the user. This will simplify the UI by avoiding the presence of two diff buttons https://github.com/tsantalis/RefactoringMiner/issues/671#issuecomment-2051144148
We have highlighting instead of connecting lines.
Additionally, when opening the editor, it should scroll to the first diff. Example: When diffing 974bf46 (#1442)., I need to scroll down.
The commit 974bf46 (#1442). is also a motivation for the arrows.
Oh, I don't see added code on the right side?
On GitHub, I see the added lines:
OK, all are green... I would like to have a "mix" of current RefactoringMiner and the GitHub diff...
@koppor
This is an interesting case in terms of diff accuracy. On the left side, there are 11 arguments, while on the right side there are 13 arguments.
This possibly means that 2 arguments are newly added, but it's a bit hard to understand what is the correct diff.
L81 seems it moved to R93. Is this correct?
L89 is now R95. Is this correct?
What are the truly newly added arguments? It seems some arguments were reordered, some added, some updated with different numbers.
This is an interesting case in terms of diff accuracy.
:-)
Therefore I was asking about arrows to check if the tool would match some intuition.
This possibly means that 2 arguments are newly added, but it's a bit hard to understand what is the correct diff.
Sure :) - I also forgot, what I added :)
L81 seems it moved to R93. Is this correct?
Yes.
L89 is now R95. Is this correct?
Yes. Fixed-test-case.
What are the truly newly added arguments? It seems some arguments were reordered,
No reordering of arguments. It should be updates only to fix the expected value at the beginning (first parameter).
- right L82 fixes left L83
- right L83 fixes left L84
- right L86 fixes left L85 (consistency to right L85)
- left L88 got lost somehow? (or could be now right L87)
- right L95 fixes left L89
@koppor
I have great news. We implemented additional diffs for moved code. This means that for a given file, there could be multiple diffs, if some methods or attributes have been moved to another class, or extracted to a new class.
For PR https://github.com/JabRef/jabref/pull/11180/files
The new diff is
JabRefFrame.java → JabRefFrameViewModel.java
@koppor I fixed the mapping of comments in PR https://github.com/JabRef/jabref/pull/11180 See issue https://github.com/tsantalis/RefactoringMiner/issues/703#issuecomment-2051108649
@koppor Happy New Year Oliver! Thank you for your valuable feedback. I wish you all the best for 2025.
I would really like to raise my wish again. Example from: https://github.com/tsantalis/RefactoringMiner/issues/894
I would like to see the diff between the two yellow-marked files - to see how they relate. That request could even trigger some backend functionality, re-evaluating a diff.