[GSOC22] - B - Implement merging fields in the three way merge UI (#9022

) * 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 * Update README.md * Fix Readme * Create grid cells for rendering field name and field value * Implement FieldNameCell and define style class * Make FieldValueCell selectable - Defined style class - Defined pseudo class for the selected state - Created the selection box UI * Implement MergedFieldCell for rendering editable merged field value * Change MergedFieldCell text based on the selected FieldValueCell * Style MergedFieldCell * Rename ThreeWayMerge css stylesheet * Create HeadView for rendering left, right and merged entry headers * Fix typo * Fix a bug that causes the selection of both left and right field cells simultaneously. - Added a style class for field value cell * Prevent the selection of FieldValueCell when it's disabled - A cell is disabled when it's either empty or not visible e.g. when the left cell spans 2 columns, right cell is disabled because it's not visible - Added comments * Display a copy icon inside FieldValueCell * Disable left or right FieldValueCell when it's empty - Added getters for row cells * Update Base CSS style * Design and implement the merge toolbox - It will be added to the top of the merge dialog * Implement the initial prototype of the new merge UI - Renamed ThreeWayMerge.java to ThreeWayMergeView * Use ThreeWayMergeView in MergeEntriesDialog - At this stage headers aren't updated and merged bib entry is not created * Fix ThreeWayMergeView.css stylesheet not being imported * Span left field value to 2 columns when left and right field values are the same * Wrap the copy icon inside a button - Updated the selection box UI * Use HAND cursor when hover over FieldValueCell * Move field cells styles to ThreeWayMergeView.css * Update merge toolbox FXML UI * Implement and style HeaderCell * Take row index as an argument in AbstractCell - Created odd and even pseudo classes that will update the background color of the cell when state changes. - Created NO_ROW_INDEX to be used by HeaderCell, because HeaderCell doesn't base its background color on its row position which is 0. * Color cell background based on its row position (odd or even) * Bind textArea.textProperty() and textProperty() using BindingsHelper * Copy field value to Clipboard user clicks on the copy button - Renamed copy-icon style class - Used ActionFactory for creating the copy icon button * Implement a more complete HeaderView - Created ColumnConstraints that matches the field grid constraints, so when the window is resized grid and header resize equally * Pass left and right headers to HeaderView * Create merged entry * 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 * Revert "[WIP] [GSOC22] Improve the external changes resolver dialog" (#9019) * Use '==' to compare enums * Move keyword merging logic to KeywordList class * Remove default constructor * Compare keywords equality without considering order * Test KeywordList#merge * Fix I10n * Fix I10n Co-authored-by: Siedlerchr <[email protected]>
JabRef · Aug 10, 2022 · dcec4e2 · dcec4e2
1 parent a90b868
commit dcec4e2
Show file tree

Hide file tree

Showing 29 changed files with 999 additions and 77 deletions.
diff --git a/src/main/java/org/jabref/gui/actions/Action.java b/src/main/java/org/jabref/gui/actions/Action.java
@@ -1,5 +1,6 @@
 package org.jabref.gui.actions;
 
+import java.util.Objects;
 import java.util.Optional;
 
 import org.jabref.gui.icon.JabRefIcon;
@@ -19,4 +20,71 @@ default Optional<KeyBinding> getKeyBinding() {
     default String getDescription() {
         return "";
     }
+
+    class Builder {
+        private final ActionImpl actionImpl;
+
+        public Builder(String text) {
+            this.actionImpl = new ActionImpl();
+            setText(text);
+        }
+
+        public Builder() {
+            this("");
+        }
+
+        public Action setIcon(JabRefIcon icon) {
+            Objects.requireNonNull(icon);
+            actionImpl.icon = icon;
+            return actionImpl;
+        }
+
+        public Action setText(String text) {
+            Objects.requireNonNull(text);
+            actionImpl.text = text;
+            return actionImpl;
+        }
+
+        public Action setKeyBinding(KeyBinding keyBinding) {
+            Objects.requireNonNull(keyBinding);
+            actionImpl.keyBinding = keyBinding;
+            return actionImpl;
+        }
+
+        public Action setDescription(String description) {
+            Objects.requireNonNull(description);
+            actionImpl.description = description;
+            return actionImpl;
+        }
+    }
+
+    class ActionImpl implements Action {
+        private JabRefIcon icon;
+        private KeyBinding keyBinding;
+        private String text;
+        private String description;
+
+        private ActionImpl() {
+        }
+
+        @Override
+        public Optional<JabRefIcon> getIcon() {
+            return Optional.ofNullable(icon);
+        }
+
+        @Override
+        public Optional<KeyBinding> getKeyBinding() {
+            return Optional.ofNullable(keyBinding);
+        }
+
+        @Override
+        public String getText() {
+            return text != null ? text : "";
+        }
+
+        @Override
+        public String getDescription() {
+            return description != null ? description : "";
+        }
+    }
 }
diff --git a/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java b/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java
@@ -138,4 +138,12 @@ private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {
     public BibEntry getMergedEntry() {
         return threeWayMerge.getMergedEntry();
     }
+
+    public BibEntry getNewLeftEntry() {
+        return threeWayMerge.getLeftEntry();
+    }
+
+    public BibEntry getNewRightEntry() {
+        return threeWayMerge.getRightEntry();
+    }
 }
diff --git a/src/main/java/org/jabref/gui/duplicationFinder/DuplicateSearch.java b/src/main/java/org/jabref/gui/duplicationFinder/DuplicateSearch.java
@@ -152,16 +152,21 @@ private void askResolveStrategy(DuplicateSearchResult result, BibEntry first, Bi
         if ((resolverResult == DuplicateResolverResult.KEEP_LEFT)
                 || (resolverResult == DuplicateResolverResult.AUTOREMOVE_EXACT)) {
             result.remove(second);
+            result.replace(first, dialog.getNewLeftEntry());
             if (resolverResult == DuplicateResolverResult.AUTOREMOVE_EXACT) {
                 autoRemoveExactDuplicates.set(true); // Remember choice
             }
         } else if (resolverResult == DuplicateResolverResult.KEEP_RIGHT) {
             result.remove(first);
+            result.replace(second, dialog.getNewRightEntry());
         } else if (resolverResult == DuplicateResolverResult.BREAK) {
             libraryAnalyzed.set(true);
             duplicates.clear();
         } else if (resolverResult == DuplicateResolverResult.KEEP_MERGE) {
             result.replace(first, second, dialog.getMergedEntry());
+        } else if (resolverResult == DuplicateResolverResult.KEEP_BOTH) {
+            result.replace(first, dialog.getNewLeftEntry());
+            result.replace(second, dialog.getNewRightEntry());
         }
     }
 
@@ -225,6 +230,11 @@ public synchronized void replace(BibEntry first, BibEntry second, BibEntry repla
             duplicates++;
         }
 
+        public synchronized void replace(BibEntry entry, BibEntry replacement) {
+            remove(entry);
+            getToAdd().add(replacement);
+        }
+
         public synchronized boolean isToRemove(BibEntry entry) {
             return toRemove.containsKey(System.identityHashCode(entry));
         }

diff --git a/src/main/java/org/jabref/gui/icon/IconTheme.java b/src/main/java/org/jabref/gui/icon/IconTheme.java
@@ -343,7 +343,9 @@ public enum JabRefIcons implements JabRefIcon {
 
         ACCEPT_LEFT(MaterialDesignS.SUBDIRECTORY_ARROW_LEFT),
 
-        ACCEPT_RIGHT(MaterialDesignS.SUBDIRECTORY_ARROW_RIGHT);
+        ACCEPT_RIGHT(MaterialDesignS.SUBDIRECTORY_ARROW_RIGHT),
+
+        MERGE_GROUPS(MaterialDesignS.SOURCE_MERGE);
 
         private final JabRefIcon icon;
 

diff --git a/src/main/java/org/jabref/gui/mergeentries/FetchAndMergeEntry.java b/src/main/java/org/jabref/gui/mergeentries/FetchAndMergeEntry.java
@@ -99,7 +99,7 @@ private void showMergeDialog(BibEntry originalEntry, BibEntry fetchedEntry, WebF
         dialog.setTitle(Localization.lang("Merge entry with %0 information", fetcher.getName()));
         dialog.setLeftHeaderText(Localization.lang("Original entry"));
         dialog.setRightHeaderText(Localization.lang("Entry from %0", fetcher.getName()));
-        Optional<BibEntry> mergedEntry = dialogService.showCustomDialogAndWait(dialog);
+        Optional<BibEntry> mergedEntry = dialogService.showCustomDialogAndWait(dialog).map(MergeResult::mergedEntry);
         if (mergedEntry.isPresent()) {
             NamedCompound ce = new NamedCompound(Localization.lang("Merge entry with %0 information", fetcher.getName()));
 

diff --git a/src/main/java/org/jabref/gui/mergeentries/MergeEntriesAction.java b/src/main/java/org/jabref/gui/mergeentries/MergeEntriesAction.java
@@ -66,28 +66,24 @@ public void execute() {
             second = one;
         }
 
-        MergeEntriesDialog dlg = new MergeEntriesDialog(first, second);
-        dlg.setTitle(Localization.lang("Merge entries"));
-        Optional<BibEntry> mergedEntry = dialogService.showCustomDialogAndWait(dlg);
-        if (mergedEntry.isPresent()) {
-            // ToDo: BibDatabase::insertEntry does not contain logic to mark the BasePanel as changed and to mark
+        MergeEntriesDialog dialog = new MergeEntriesDialog(first, second);
+        dialog.setTitle(Localization.lang("Merge entries"));
+        Optional<MergeResult> mergeResultOpt = dialogService.showCustomDialogAndWait(dialog);
+        mergeResultOpt.ifPresentOrElse(mergeResult -> {
+            // TODO: BibDatabase::insertEntry does not contain logic to mark the BasePanel as changed and to mark
             //  entries with a timestamp, only BasePanel::insertEntry does. Workaround for the moment is to get the
             //  BasePanel from the constructor injected JabRefFrame. Should be refactored and extracted!
-            frame.getCurrentLibraryTab().insertEntry(mergedEntry.get());
+            frame.getCurrentLibraryTab().insertEntry(mergeResult.mergedEntry());
 
-            // Create a new entry and add it to the undo stack
-            // Remove the other two entries and add them to the undo stack (which is not working...)
             NamedCompound ce = new NamedCompound(Localization.lang("Merge entries"));
-            ce.addEdit(new UndoableInsertEntries(databaseContext.getDatabase(), mergedEntry.get()));
+            ce.addEdit(new UndoableInsertEntries(databaseContext.getDatabase(), mergeResult.mergedEntry()));
             List<BibEntry> entriesToRemove = Arrays.asList(one, two);
             ce.addEdit(new UndoableRemoveEntries(databaseContext.getDatabase(), entriesToRemove));
             databaseContext.getDatabase().removeEntries(entriesToRemove);
             ce.end();
-            Globals.undoManager.addEdit(ce); // ToDo: Rework UndoManager and extract Globals
+            Globals.undoManager.addEdit(ce);
 
             dialogService.notify(Localization.lang("Merged entries"));
-        } else {
-            dialogService.notify(Localization.lang("Canceled merging entries"));
-        }
+        }, () -> dialogService.notify(Localization.lang("Canceled merging entries")));
     }
 }
diff --git a/src/main/java/org/jabref/gui/mergeentries/MergeEntriesDialog.java b/src/main/java/org/jabref/gui/mergeentries/MergeEntriesDialog.java
@@ -8,7 +8,7 @@
 import org.jabref.logic.l10n.Localization;
 import org.jabref.model.entry.BibEntry;
 
-public class MergeEntriesDialog extends BaseDialog<BibEntry> {
+public class MergeEntriesDialog extends BaseDialog<MergeResult> {
     private final ThreeWayMergeView threeWayMergeView;
 
     public MergeEntriesDialog(BibEntry one, BibEntry two) {
@@ -31,7 +31,7 @@ private void init() {
         this.getDialogPane().getButtonTypes().setAll(ButtonType.CANCEL, replaceEntries);
         this.setResultConverter(buttonType -> {
             if (buttonType.equals(replaceEntries)) {
-                return threeWayMergeView.getMergedEntry();
+                return new MergeResult(threeWayMergeView.getLeftEntry(), threeWayMergeView.getRightEntry(), threeWayMergeView.getMergedEntry());
             } else {
                 return null;
             }

diff --git a/src/main/java/org/jabref/gui/mergeentries/MergeResult.java b/src/main/java/org/jabref/gui/mergeentries/MergeResult.java
@@ -0,0 +1,8 @@
+package org.jabref.gui.mergeentries;
+
+import org.jabref.model.entry.BibEntry;
+
+public record MergeResult(
+        BibEntry leftEntry, BibEntry rightEntry, BibEntry mergedEntry
+) {
+}