[WIP][GSOC22] - C - Improve the external changes resolver dialog (#9021)

* Add the entry type field at the top of the fields list

- Sorted fields by name
- Removed internal fields

* Display entry type and bind merged entry

* Minor changes

* Style CSS

* Update ThreeWayMergeToolbox.fxml

* Remove MergeEntries from MergeEntriesDialog

* Create EmptyCell for optimization

- When cell is empty there is no point of creating a label object

* Update HeaderCell

* Rename HeaderView to ThreeWayMergeHeaderView

* Add buttons to accept all left or right entry changes to ThreeWayMergeToolbar

* Disable right field cell when it's not visible

* Listen for select all left/right toolbar buttons

* Style stuff

* Add DiffHighlighter

- Implemented UnifiedDiffHighlighter

* Create DiffHighlighter.css

* Add left padding to HeaderCell

* Move diff highlighting styles to Dark.css

- Just for testing

* Show differences between left and right values using a unified diff view

- Just for testing

* Change MergeEntriesDialog stage style

* Remove BackgroundTone from AbstractCell

* Move CopyFieldValueCommand

* Make DiffMethod public

* Move CopyFieldValueCommand

* Implement SplitDiffHighlighter

* Show diffs when user selects "Show Diff" in the toolbar UI

- Only unified diff view will be shown for now

* Fix bugs in UnifiedDiffHighlighter

* Style css stuff

* Fix typos

* Refactor SplitDiffHighlighter and fix bugs

- Added "updated" style class for styling CHANGE diffs

* Remove redundant constructor from UnifiedDiffHighlighter

* Allow users to switch between split and unified diff view

- Fixed some bugs

* Set left and right headers in MergeEntriesDialog

* Refactor ThreeWayMergeHeaderView

* Remove wellbehavedfx module dependency

* Delete EmptyCell.java

- I might recreate it later, but for now, I'm choosing simplicity over performance

* Refactor FieldValueCell

* Use the new merge UI in DuplicateResolverDialog

* Expose an API to change diff view and highlight method from outside the merge toolbar

* Use the new merge UI when an entry is changed from external

* Minor styling

* Remove DiffHighlighter.css stylesheet

- You can find diff styles in Base.css and Dark.css

* Remove unused dependencies

* Allow selecting empty field values

* Fix readme :(

* Use a theme color for selection box background

* Refactor FieldValueCell

* Add an action to FieldValueCell for opening Urls

* Add the ability to open DOI identifier

* Cleanup

* Checkstyle

* Improve URL validation logic

* Use DiffMethod in merge toolbar for consistency

* i18n

# Conflicts:
#	src/main/resources/l10n/JabRef_en.properties

* Cleanup css

* Remove redundant icon style

* Rename style class field-value to merge-field-value

- Since it is used in Base.css and Dark.css, it is critical to distinguish the field cell used is related to the merge UI.

* Remove old Merge entries UI

* Fix missing header styles

* i18n

# Conflicts:
#	src/main/resources/l10n/JabRef_en.properties

* Garbage collect right cell when it's invisible

- When both the left and right cells have the same value, only the left value is displayed, making it unnecessary to keep allocating memory for the right cell.

* Fix typo

* Create GroupsFieldNameCell

* Create MergedGroups to record the state of the groups before and after the merge

* Create a factory class for creating field name cells

* Perform experiments for implementing groups merging

* Delegate field name cells creation to ThreeWayMergeView

- Seperated field values and field name cells because now it's easier to update field values cells without touching the field name cell

* Improve groups merging function

* Implement updateFieldValues to redraw the field row when left or right entry changes

* Write a draft version of the logic to merge groups

* Refactoring

* More Refactoring

* Merge groups only when left and right entries have different groups

* Add row constraints to fix UI lagging when updating field values

* Minor refactoring

* Don't merge common groups between left and right entries

* Refactor GroupsFieldNameCell constructor

* Allow groups field row to grow in height

* Fix select all left/right not working properly

* Update diff when a field value changes

* Add the merge icon and style css

* Extract merge and unmerge commands into their own classes

* Refactoring

* Record groups merging operation using CompoundEdit

- Exposed a public API to be able to cancel groups merge

* Cancel groups merge when user choose 'Cancel' in the merge entries dialog

* Make FieldNameCell take only one action

* Set groups merge action to MERGE by default

* Fix Unmerge button always disabled

* Cleanup

* Almost f**** everything up. I deleted GroupsFieldNameCell content by mistake, it's all ok now

- I did a rebase and then when resolving conflicts, I chose "Accept Theirs" all the way but that didn't go well, now I have no history of GroupsFieldNameCell

* Checkstyle

* Remove unused MergedGroups class

- Because now groups merge history is kept in a compound  Edit

* Register groups merge edit into UndoManager

* Cancel merge groups edit in the DuplicateResolverDialog when user choose 'Cancel' button

* i18n

# Conflicts:
#	src/main/resources/l10n/JabRef_en.properties

* Introduce ThreeFieldValuesViewModel

* Merge clones of left & right entry rather than the original ones

* Optimize imports

* Refactor DuplicateResolverDialog to merge clones of left and right entry

* Select the other entry groups when one entry has no group

* Remove unused property

* [WIP] Convert ThreeFieldValues to MVVM

- Together with my mentors, we decided to convert ThreeFieldValues to MVVM in order to maintain the codebase consistency and facilitate testing. The view is ThreeFieldValues, and it has a view model.

* Move business logic to the view-model and update UI via binding

* Consider equal left and right fields as merged

* Move initial selection code to the view model

- Replaced addListener() with EasyBind#subscribe

* Refactor

* Hide rightValueCell and extend leftValueCell to 2 columns when fields are merged

* Prepare for adding the ability to merge fields other than groups

* Rename ThreeFieldValues to ThreeFieldValuesView

* Delegate FieldNameCell creation to ThreeFieldValuesView

* Pass Bib entries to ThreeFieldValuesView rather than raw string

- Because I want to add the merge/unmerge commands to ThreeFieldValuesView and I need to have a reference to left and right bib entries

* Move merge/unmerge commands to ThreeFieldValuesView

* Bind field value cell text property

* Create an undoable edit for undoing fields merging

* Update bib entries when left and right field value properties change

* Update left and right field value properties when merging fields rather than the bib entries

* Unbind MergedFieldValue property because it can't be edited when bound

* Cleanup

* Move merged entry update logic to ThreeFieldValuesViewModel

* Implement algorithms for merging groups and keywords

* Pass FieldMergerFactory to ThreeFieldValuesView

* Move isMergeableField method to FieldMergerFactory

* Delete cancelGroupsEdit()

* Fix KeywordMerger implementation

* Fix a bug that deselects both fields after they are unmerged

* Rename 'ThreeFieldValues' to 'FieldRow'

* Cleanup

* Hide diffs when both values are equal

* Fix typos

* Select LEFT value after merging fields

* Logging

* Implement merging comments

* Improve groups and keywords merger implementation

* Change merge/unmerge buttons tooltip text based on the field to merge

* Implement file merging

* Encode the resulting string from merging files

* Refactor merge/unmerge fields command handling

* Declare Actions with the builder design pattern

* Remove MergeableFieldCell dependency from the merge/unmerge commands

- Thus commands can be moved to the view model

* Add logs

* Disable field's merge button when field values are equal

* Update code documentation

* Rename 'AbstractCell' to 'ThreeWayMergeCell'

* Pass arguments to Localization.lang using '%0' not '%'

* Decompose the ToggleMergeUnmergeButton into its own class (or component)

- Removed FieldNameCellFactory.java and MergeableFieldCell.java
- Made it easier to add more side buttons in the future

* Include original left and right entries in EntriesMergeResult

* Make entries merging logic reusable

* Rename ChangeDisplayDialog.java to ExternalChangesResolverDialog.java

* Create ExternalChangesResolverViewModel

* Design the ExternalChangesResolverDialog

* Implement a method for opening an advanced merge dialog for modified entries

* Wrap the UI in a dialog pane

* Convert change name to a StringProperty instead of a raw string

* Populate table view with changes

- Initialized the change name column
- Commented out 'getDialogPane().setContent(pane)' because it will be removed later

* Remove 'Accept changes' and 'Dismiss' buttons from the dialog

* Allow selecting multiple changes

* Rename 'Accept Theirs' to 'Accept' and 'Accept Yours' to 'Deny'

* Apply changes when before the dialog

- Changes are applied only when all changes are resolved (changes table is empty). Like a transaction

* Cleanup FXML

* Remove old UI code

* Close dialog when all changes are resolved

* Enable the 'Merge...' button only if the selected change has an advanced merge dialog

* Select the first change after opening the dialog

* Open the advanced merge dialog when 'Merge...' is clicked

* Don't open the advanced merge dialog when 'makeChange()' is called on 'EntryChangeViewModel'

* Display a preview of the deleted entry

* Display information about the selected change in the bottom detail node

* Use Infinity in FXML instead of an exponential number

* Inject dependencies in constructor

* Pass parameters to 'Localization.lang' instead of concatenation

* Rename change column from 'Name' to 'Change'

* Remove obsolete localization keys

* Preview the new entry state in the details pane when an entry is modified from external

* Register advanced changes made from the advanced merge dialog

* Use to accept property in DatabaseChangeViewModel to decide which changes to accept

* Use 'BibDatabase#insertEntry' instead of 'JabRefFrame#insert' to insert an entry

- I noticed a comment that said that the library won't be marked as changed (*) when inserting entries using BibDatabase#insertEntry, but the comment seems to be out of date. Hopefully this doesn't change any behavior.

* Don't override 'setAccepted()' in EntryChangeViewModel

* Remove unused JabRefFrame constructor argument

* Convert ExternalChangesResolverDialog to MVVM

* Select first change

* Configure advanced entry change resolver/merger dialog

* Progress toward converting the collab package to MVVM

* Move experimental code into the same package

* Commit

* Checkstyle

* Checkstyle

* l10n

* Remove FieldRowController.java

* Remove the selected changes from the view model to prevent bugs

- Set selection mode to SINGLE

* Don't pass the selected change because it is already available in the view model

* Fix change produced by the advanced merge dialog not applied

* Cleanup

* [WIP] Refactor the external change resolver component

* [WIP] still working

* Rename 'changes' to 'visibleChanges' and 'immutableChanges' to 'changes'

* Pass 'oldEntry' and 'newEntry' to EntryChange

* Initialize the preview of an Entry change

* Prevent NullPointerException when the change has no resolver

* Implement EntryChangeResolver#askUserToResolveChange

* Cleanup

* Pass EntryChange to EntryChangeResolver

* Add accept() method to ExternalChange

* Integrate the experiment design into ExternalChangesResolverDialog

* pass undoEdit to applyChange method

* Make ExternalChangeResolver#askUserToResolveChange return an Optional

* Fix compilation errors by replacing DatabaseChangeViewModel by ExternalChange

* Remove undoEdit field from ExternalChangeResolver

* Pass a BibDatabaseContext to ExternalChangeResolverFactory

* Pass required dependencies to ExternalChangeDetailsViewFactory

* Fix typo

* Remove debugging junk

* Removed unused import

* Remove EntryChangeDetailsViewModel.java because it's useless

* Handle EntryAdd changes in the new design

* Cleanup

* Set dialog title and cleanup

* Integrate EntryDelete changes into the new design

* Integrate StringAdd changes into the new design

* Integrate StringDelete changes into the new design

* Integrate StringChange changes into the new design

* Integrate StringRename changes into the new design

* Integrate MetadataChange changes into the new design

* Integrate GroupChange changes into the new design

* Integrate PreambleChange changes into the new design

* It's done 🎉

* I10n

* Fix typo in registering entry change edits

* Register edits into UndoManager

* Add a TODO for undoing metadata edit

* Log the number of cached web view instances in WebViewStore

* Cache external change details views

 Reconstructing the details view to preview an {@link ExternalChange} every time it's selected is a heavy operation. It is also useless because changes are static and if the change data is static then the view doesn't have to change either. This cache is used to ensure that we only create the detail view instance once for each {@link ExternalChange}.

* Update CHANGELOG.md

* Refine dialog size and divider position

* Update src/main/java/org/jabref/gui/collab/stringadd/StringAdd.java

Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>

* Use parametrized localization

* Remove Undo

* Use computeIfAbsent to lookup cached details views

* Rename 'String' to 'BibTexString'

* Update src/main/java/org/jabref/gui/collab/ExternalChange.java

Co-authored-by: Christoph <siedlerkiller@gmail.com>

* Use debug() for logging events not of interest for the average user

* Remove ExternalChangeDetailsViewModel.java

CHANGELOG.md

@@ -23,6 +23,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We call backup files `.bak` and temporary writing files now `.sav`.
 - JabRef keeps 10 older versions of a `.bib` file in the [user data dir](https://github.com/harawata/appdirs#supported-directories) (instead of a single `.sav` (now: `.bak`) file in the directory of the `.bib` file)
 - We changed the button label from "Return to JabRef" to "Return to library" to better indicate the purpose of the action.
+- We reworked the External Changes Resolver dialog. [#9021](https://github.com/JabRef/jabref/pull/9021)
 
 ### Fixed
 

src/main/java/org/jabref/gui/JabRefFrame.java

@@ -827,7 +827,7 @@ private MenuBar createMenu() {
 
         quality.getItems().addAll(
                 factory.createMenuItem(StandardActions.FIND_DUPLICATES, new DuplicateSearch(this, dialogService, stateManager)),
-                factory.createMenuItem(StandardActions.MERGE_ENTRIES, new MergeEntriesAction(this, dialogService, stateManager)),
+                factory.createMenuItem(StandardActions.MERGE_ENTRIES, new MergeEntriesAction(dialogService, stateManager)),
                 factory.createMenuItem(StandardActions.CHECK_INTEGRITY, new IntegrityCheckAction(this, stateManager, Globals.TASK_EXECUTOR)),
                 factory.createMenuItem(StandardActions.CLEANUP_ENTRIES, new CleanupAction(this, this.prefs, dialogService, stateManager)),
 

src/main/java/org/jabref/gui/collab/ChangeScanner.java

@@ -7,6 +7,16 @@
 
 import org.jabref.gui.DialogService;
 import org.jabref.gui.StateManager;
+import org.jabref.gui.collab.entryadd.EntryAdd;
+import org.jabref.gui.collab.entrychange.EntryChange;
+import org.jabref.gui.collab.entrydelete.EntryDelete;
+import org.jabref.gui.collab.groupchange.GroupChange;
+import org.jabref.gui.collab.metedatachange.MetadataChange;
+import org.jabref.gui.collab.preamblechange.PreambleChange;
+import org.jabref.gui.collab.stringadd.BibTexStringAdd;
+import org.jabref.gui.collab.stringchange.BibTexStringChange;
+import org.jabref.gui.collab.stringdelete.BibTexStringDelete;
+import org.jabref.gui.collab.stringrename.BibTexStringRename;
 import org.jabref.gui.theme.ThemeManager;
 import org.jabref.logic.bibtex.comparator.BibDatabaseDiff;
 import org.jabref.logic.bibtex.comparator.BibEntryDiff;
@@ -31,6 +41,8 @@ public class ChangeScanner {
     private final StateManager stateManager;
     private final ThemeManager themeManager;
 
+    private final ExternalChangeResolverFactory externalChangeResolverFactory;
+
     public ChangeScanner(BibDatabaseContext database,
                          DialogService dialogService,
                          PreferencesService preferencesService,
@@ -41,15 +53,16 @@ public ChangeScanner(BibDatabaseContext database,
         this.preferencesService = preferencesService;
         this.stateManager = stateManager;
         this.themeManager = themeManager;
+        this.externalChangeResolverFactory = new ExternalChangeResolverFactory(dialogService, database);
     }
 
-    public List<DatabaseChangeViewModel> scanForChanges() {
+    public List<ExternalChange> scanForChanges() {
         if (database.getDatabasePath().isEmpty()) {
             return Collections.emptyList();
         }
 
         try {
-            List<DatabaseChangeViewModel> changes = new ArrayList<>();
+            List<ExternalChange> changes = new ArrayList<>();
 
             // Parse the modified file
             // Important: apply all post-load actions
@@ -61,10 +74,12 @@ public List<DatabaseChangeViewModel> scanForChanges() {
             // Start looking at changes.
             BibDatabaseDiff differences = BibDatabaseDiff.compare(database, databaseOnDisk);
             differences.getMetaDataDifferences().ifPresent(diff -> {
-                changes.add(new MetaDataChangeViewModel(diff, preferencesService));
-                diff.getGroupDifferences().ifPresent(groupDiff -> changes.add(new GroupChangeViewModel(groupDiff)));
+                changes.add(new MetadataChange(diff, database, externalChangeResolverFactory));
+                diff.getGroupDifferences().ifPresent(groupDiff -> changes.add(new GroupChange(
+                        groupDiff, database, externalChangeResolverFactory
+                )));
             });
-            differences.getPreambleDifferences().ifPresent(diff -> changes.add(new PreambleChangeViewModel(diff)));
+            differences.getPreambleDifferences().ifPresent(diff -> changes.add(new PreambleChange(diff, database, externalChangeResolverFactory)));
             differences.getBibStringDifferences().forEach(diff -> changes.add(createBibStringDiff(diff)));
             differences.getEntryDifferences().forEach(diff -> changes.add(createBibEntryDiff(diff)));
             return changes;
@@ -74,31 +89,31 @@ public List<DatabaseChangeViewModel> scanForChanges() {
         }
     }
 
-    private DatabaseChangeViewModel createBibStringDiff(BibStringDiff diff) {
-        if (diff.getOriginalString() == null) {
-            return new StringAddChangeViewModel(diff.getNewString());
+    private ExternalChange createBibStringDiff(BibStringDiff diff) {
+         if (diff.getOriginalString() == null) {
+            return new BibTexStringAdd(diff.getNewString(), database, externalChangeResolverFactory);
         }
 
         if (diff.getNewString() == null) {
-            return new StringRemoveChangeViewModel(diff.getOriginalString());
+            return new BibTexStringDelete(diff.getOriginalString(), database, externalChangeResolverFactory);
         }
 
         if (diff.getOriginalString().getName().equals(diff.getNewString().getName())) {
-            return new StringChangeViewModel(diff.getOriginalString(), diff.getNewString());
+            return new BibTexStringChange(diff.getOriginalString(), diff.getNewString(), database, externalChangeResolverFactory);
         }
 
-        return new StringNameChangeViewModel(diff.getOriginalString(), diff.getNewString());
+        return new BibTexStringRename(diff.getOriginalString(), diff.getNewString(), database, externalChangeResolverFactory);
     }
 
-    private DatabaseChangeViewModel createBibEntryDiff(BibEntryDiff diff) {
+    private ExternalChange createBibEntryDiff(BibEntryDiff diff) {
         if (diff.getOriginalEntry() == null) {
-            return new EntryAddChangeViewModel(diff.getNewEntry(), preferencesService, dialogService, stateManager, themeManager);
+            return new EntryAdd(diff.getNewEntry(), database, externalChangeResolverFactory);
         }
 
         if (diff.getNewEntry() == null) {
-            return new EntryDeleteChangeViewModel(diff.getOriginalEntry());
+            return new EntryDelete(diff.getOriginalEntry(), database, externalChangeResolverFactory);
         }
 
-        return new EntryChangeViewModel(diff.getOriginalEntry(), diff.getNewEntry());
+        return new EntryChange(diff.getOriginalEntry(), diff.getNewEntry(), database, externalChangeResolverFactory);
     }
 }

src/main/java/org/jabref/gui/collab/DatabaseChangeListener.java

@@ -3,5 +3,5 @@
 import java.util.List;
 
 public interface DatabaseChangeListener {
-    void databaseChanged(List<DatabaseChangeViewModel> changes);
+    void databaseChanged(List<ExternalChange> changes);
 }

src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java

@@ -67,7 +67,7 @@ public DatabaseChangeMonitor(BibDatabaseContext database,
                 Localization.lang("The library has been modified by another program."),
                 List.of(new Action(Localization.lang("Dismiss changes"), event -> notificationPane.hide()),
                         new Action(Localization.lang("Review changes"), event -> {
-                            dialogService.showCustomDialogAndWait(new ChangeDisplayDialog(database, changes));
+                            dialogService.showCustomDialogAndWait(new ExternalChangesResolverDialog(changes, database, dialogService, stateManager, themeManager, preferencesService));
                             notificationPane.hide();
                         })),
                 Duration.ZERO));