Added option to treat all duplicate entries in the same way (#10741)

* Added option to treat all duplicate entries in the same way add checkbox * change to menu button * Store and read decision in the prefeerences * fix default * check decision * checkstyle * checkstyle * add preferences checkbox sync * fix space * return keep left on cancel fix casing * add combobox to prefs * fix l10n * Add default translations and show in prefs * When copying only ask for the first entry * Fix weird copy with jump to key as well Fixes #9169 * refactor * add l10n * refactor, use checkboxes instead of menu * Fix prefs not stored * l10n * Fix bug with opening eprint * Extract MergeDialogPreferences * checkstyle * remove checkbox stuff in preferences * l10n --------- Co-authored-by: Oliver Kopp <[email protected]>
JabRef · Jan 15, 2024 · 899f503 · 899f503
1 parent 7c01040
commit 899f503
Show file tree

Hide file tree

Showing 19 changed files with 358 additions and 205 deletions.
diff --git a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java
@@ -32,6 +32,10 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.jabref.model.entry.field.StandardField.PDF;
+import static org.jabref.model.entry.field.StandardField.PS;
+import static org.jabref.model.entry.field.StandardField.URL;
+
 /**
  * See http://stackoverflow.com/questions/18004150/desktop-api-is-not-supported-on-the-current-platform for more implementation hints.
  * http://docs.oracle.com/javase/7/docs/api/java/awt/Desktop.html cannot be used as we don't want to rely on AWT
@@ -60,7 +64,7 @@ public static void openExternalViewer(BibDatabaseContext databaseContext,
             throws IOException {
         String link = initialLink;
         Field field = initialField;
-        if ((StandardField.PS == field) || (StandardField.PDF == field)) {
+        if ((PS == field) || (PDF == field)) {
             // Find the default directory for this field type:
             List<Path> directories = databaseContext.getFileDirectories(preferencesService.getFilePreferences());
 
@@ -76,10 +80,10 @@ public static void openExternalViewer(BibDatabaseContext databaseContext,
             String[] split = file.get().getFileName().toString().split("\\.");
             if (split.length >= 2) {
                 if ("pdf".equalsIgnoreCase(split[split.length - 1])) {
-                    field = StandardField.PDF;
+                    field = PDF;
                 } else if ("ps".equalsIgnoreCase(split[split.length - 1])
                         || ((split.length >= 3) && "ps".equalsIgnoreCase(split[split.length - 2]))) {
-                    field = StandardField.PS;
+                    field = PS;
                 }
             }
         } else if (StandardField.DOI == field) {
@@ -102,29 +106,32 @@ public static void openExternalViewer(BibDatabaseContext databaseContext,
                 if (eprintTypeOpt.isEmpty() && archivePrefixOpt.isEmpty()) {
                     dialogService.showErrorDialogAndWait(Localization.lang("Unable to open linked eprint. Please set the eprinttype field"));
                 } else {
-                    dialogService.showErrorDialogAndWait(Localization.lang("Unable to open linked eprint. Please verify that the eprint field has a valid '%0' id", eprintTypeOpt.get()));
+                    dialogService.showErrorDialogAndWait(Localization.lang("Unable to open linked eprint. Please verify that the eprint field has a valid '%0' id", link));
                 }
             }
             // should be opened in browser
-            field = StandardField.URL;
+            field = URL;
         }
 
-        if (StandardField.URL == field) {
-            openBrowser(link, preferencesService.getFilePreferences());
-        } else if (StandardField.PS == field) {
-            try {
-                NATIVE_DESKTOP.openFile(link, StandardField.PS.getName(), preferencesService.getFilePreferences());
-            } catch (IOException e) {
-                LOGGER.error("An error occurred on the command: " + link, e);
+        switch (field) {
+            case URL ->
+                    openBrowser(link, preferencesService.getFilePreferences());
+            case PS -> {
+                try {
+                    NATIVE_DESKTOP.openFile(link, PS.getName(), preferencesService.getFilePreferences());
+                } catch (IOException e) {
+                    LOGGER.error("An error occurred on the command: " + link, e);
+                }
             }
-        } else if (StandardField.PDF == field) {
-            try {
-                NATIVE_DESKTOP.openFile(link, StandardField.PDF.getName(), preferencesService.getFilePreferences());
-            } catch (IOException e) {
-                LOGGER.error("An error occurred on the command: " + link, e);
+            case PDF -> {
+                try {
+                    NATIVE_DESKTOP.openFile(link, PDF.getName(), preferencesService.getFilePreferences());
+                } catch (IOException e) {
+                    LOGGER.error("An error occurred on the command: " + link, e);
+                }
             }
-        } else {
-            LOGGER.info("Message: currently only PDF, PS and HTML files can be opened by double clicking");
+            case null, default ->
+                    LOGGER.info("Message: currently only PDF, PS and HTML files can be opened by double clicking");
         }
     }
 

diff --git a/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java b/src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java
@@ -33,12 +33,30 @@ public enum DuplicateResolverType {
     }
 
     public enum DuplicateResolverResult {
-        KEEP_BOTH,
-        KEEP_LEFT,
-        KEEP_RIGHT,
-        AUTOREMOVE_EXACT,
-        KEEP_MERGE,
-        BREAK
+        KEEP_BOTH(Localization.lang("Keep both")),
+        KEEP_LEFT(Localization.lang("Keep existing entry")),
+        KEEP_RIGHT(Localization.lang("Keep from import")),
+        AUTOREMOVE_EXACT(Localization.lang("Automatically remove exact duplicates")),
+        KEEP_MERGE(Localization.lang("Keep merged")),
+        BREAK(Localization.lang("Ask every time"));
+
+        final String defaultTranslationForImport;
+
+        DuplicateResolverResult(String defaultTranslationForImport) {
+            this.defaultTranslationForImport = defaultTranslationForImport;
+        }
+
+        public String getDefaultTranslationForImport() {
+            return defaultTranslationForImport;
+        }
+
+        public static DuplicateResolverResult parse(String name) {
+            try {
+                return DuplicateResolverResult.valueOf(name);
+            } catch (IllegalArgumentException e) {
+                return BREAK; // default
+            }
+        }
     }
 
     private ThreeWayMergeView threeWayMerge;
@@ -87,10 +105,10 @@ private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {
                 threeWayMerge = new ThreeWayMergeView(one, two, preferencesService);
             }
             case IMPORT_CHECK -> {
-                first = new ButtonType(Localization.lang("Keep old entry"), ButtonData.LEFT);
+                first = new ButtonType(Localization.lang("Keep existing entry"), ButtonData.LEFT);
                 second = new ButtonType(Localization.lang("Keep from import"), ButtonData.LEFT);
                 both = new ButtonType(Localization.lang("Keep both"), ButtonData.LEFT);
-                threeWayMerge = new ThreeWayMergeView(one, two, Localization.lang("Old entry"),
+                threeWayMerge = new ThreeWayMergeView(one, two, Localization.lang("Existing entry"),
                         Localization.lang("From import"), preferencesService);
             }
             default -> throw new IllegalStateException("Switch expression should be exhaustive");
@@ -134,6 +152,8 @@ private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {
                 return DuplicateResolverResult.KEEP_MERGE;
             } else if (button.equals(removeExact)) {
                 return DuplicateResolverResult.AUTOREMOVE_EXACT;
+            } else if (button.equals(cancel)) {
+                return DuplicateResolverResult.KEEP_LEFT;
             }
             return null;
         });

diff --git a/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java b/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java
@@ -56,6 +56,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.jabref.gui.duplicationFinder.DuplicateResolverDialog.DuplicateResolverResult.BREAK;
+
 public class ImportHandler {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(ImportHandler.class);
@@ -193,10 +195,14 @@ public void importCleanedEntries(List<BibEntry> entries) {
     }
 
     public void importEntryWithDuplicateCheck(BibDatabaseContext bibDatabaseContext, BibEntry entry) {
+        importEntryWithDuplicateCheck(bibDatabaseContext, entry, BREAK);
+    }
+
+    private void importEntryWithDuplicateCheck(BibDatabaseContext bibDatabaseContext, BibEntry entry, DuplicateResolverDialog.DuplicateResolverResult decision) {
         BibEntry entryToInsert = cleanUpEntry(bibDatabaseContext, entry);
         Optional<BibEntry> existingDuplicateInLibrary = findDuplicate(bibDatabaseContext, entryToInsert);
         if (existingDuplicateInLibrary.isPresent()) {
-            Optional<BibEntry> duplicateHandledEntry = handleDuplicates(bibDatabaseContext, entryToInsert, existingDuplicateInLibrary.get());
+            Optional<BibEntry> duplicateHandledEntry = handleDuplicates(bibDatabaseContext, entryToInsert, existingDuplicateInLibrary.get(), decision);
             if (duplicateHandledEntry.isEmpty()) {
                 return;
             }
@@ -216,8 +222,8 @@ public Optional<BibEntry> findDuplicate(BibDatabaseContext bibDatabaseContext, B
         return new DuplicateCheck(Globals.entryTypesManager).containsDuplicate(bibDatabaseContext.getDatabase(), entryToCheck, bibDatabaseContext.getMode());
     }
 
-    public Optional<BibEntry> handleDuplicates(BibDatabaseContext bibDatabaseContext, BibEntry originalEntry, BibEntry duplicateEntry) {
-        DuplicateDecisionResult decisionResult = getDuplicateDecision(originalEntry, duplicateEntry, bibDatabaseContext);
+    public Optional<BibEntry> handleDuplicates(BibDatabaseContext bibDatabaseContext, BibEntry originalEntry, BibEntry duplicateEntry, DuplicateResolverDialog.DuplicateResolverResult decision) {
+        DuplicateDecisionResult decisionResult = getDuplicateDecision(originalEntry, duplicateEntry, bibDatabaseContext, decision);
         switch (decisionResult.decision()) {
             case KEEP_RIGHT:
                 bibDatabaseContext.getDatabase().removeEntry(duplicateEntry);
@@ -236,9 +242,14 @@ public Optional<BibEntry> handleDuplicates(BibDatabaseContext bibDatabaseContext
         return Optional.of(originalEntry);
     }
 
-    public DuplicateDecisionResult getDuplicateDecision(BibEntry originalEntry, BibEntry duplicateEntry, BibDatabaseContext bibDatabaseContext) {
+    public DuplicateDecisionResult getDuplicateDecision(BibEntry originalEntry, BibEntry duplicateEntry, BibDatabaseContext bibDatabaseContext, DuplicateResolverDialog.DuplicateResolverResult decision) {
         DuplicateResolverDialog dialog = new DuplicateResolverDialog(duplicateEntry, originalEntry, DuplicateResolverDialog.DuplicateResolverType.IMPORT_CHECK, bibDatabaseContext, stateManager, dialogService, preferencesService);
-        DuplicateResolverDialog.DuplicateResolverResult decision = dialogService.showCustomDialogAndWait(dialog).orElse(DuplicateResolverDialog.DuplicateResolverResult.BREAK);
+        if (decision == BREAK) {
+            decision = dialogService.showCustomDialogAndWait(dialog).orElse(BREAK);
+        }
+        if (preferencesService.getMergeDialogPreferences().shouldMergeApplyToAllEntries()) {
+            preferencesService.getMergeDialogPreferences().setAllEntriesDuplicateResolverDecision(decision);
+        }
         return new DuplicateDecisionResult(decision, dialog.getMergedEntry());
     }
 
@@ -253,17 +264,17 @@ public void setAutomaticFields(List<BibEntry> entries) {
     public void downloadLinkedFiles(BibEntry entry) {
         if (preferencesService.getFilePreferences().shouldDownloadLinkedFiles()) {
             entry.getFiles().stream()
-                    .filter(LinkedFile::isOnlineLink)
-                    .forEach(linkedFile ->
-                            new LinkedFileViewModel(
-                                    linkedFile,
-                                    entry,
-                                    bibDatabaseContext,
-                                    taskExecutor,
-                                    dialogService,
-                                    preferencesService
-                            ).download()
-                    );
+                 .filter(LinkedFile::isOnlineLink)
+                 .forEach(linkedFile ->
+                         new LinkedFileViewModel(
+                                 linkedFile,
+                                 entry,
+                                 bibDatabaseContext,
+                                 taskExecutor,
+                                 dialogService,
+                                 preferencesService
+                         ).download()
+                 );
         }
     }
 
@@ -361,4 +372,24 @@ private List<BibEntry> fetchByISBN(ISBN isbn) throws FetcherException {
         Optional<BibEntry> entry = new IsbnFetcher(preferencesService.getImportFormatPreferences()).performSearchById(isbn.getNormalized());
         return OptionalUtil.toList(entry);
     }
+
+    public void importEntriesWithDuplicateCheck(BibDatabaseContext database, List<BibEntry> entriesToAdd) {
+        boolean firstEntry = true;
+        for (BibEntry entry : entriesToAdd) {
+            if (firstEntry) {
+                LOGGER.debug("First entry to import, we use BREAK");
+                importEntryWithDuplicateCheck(database, entry, BREAK);
+                firstEntry = false;
+                continue;
+            }
+            if (preferencesService.getMergeDialogPreferences().shouldMergeApplyToAllEntries()) {
+                var decision = preferencesService.getMergeDialogPreferences().getAllEntriesDuplicateResolverDecision();
+                LOGGER.debug("Not first entry, pref flag is true, we use {}", decision);
+                importEntryWithDuplicateCheck(database, entry, decision);
+            } else {
+                LOGGER.debug("not first entry, not pref flag, break will  be used");
+                importEntryWithDuplicateCheck(database, entry);
+            }
+        }
+    }
 }
diff --git a/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java b/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java
@@ -118,11 +118,7 @@ private void initialize() {
                         return database.getDBMSSynchronizer().getDBName() + " [" + Localization.lang("shared") + "]";
                     }
 
-                    if (dbOpt.isEmpty()) {
-                        return Localization.lang("untitled");
-                    }
-
-                    return dbOpt.get();
+                    return dbOpt.orElseGet(() -> Localization.lang("untitled"));
                 })
                 .install(libraryListView);
         viewModel.selectedDbProperty().bind(libraryListView.getSelectionModel().selectedItemProperty());

diff --git a/src/main/java/org/jabref/gui/maintable/MainTable.java b/src/main/java/org/jabref/gui/maintable/MainTable.java
@@ -186,7 +186,7 @@ public MainTable(MainTableDataModel model,
             if (this.getSortOrder().isEmpty()) {
                 return;
             }
-            this.jumpToSearchKey(getSortOrder().get(0), key);
+            this.jumpToSearchKey(getSortOrder().getFirst(), key);
         });
 
         database.getDatabase().registerListener(this);
@@ -206,11 +206,15 @@ public MainTable(MainTableDataModel model,
 
     /**
      * This is called, if a user starts typing some characters into the keyboard with focus on main table. The {@link MainTable} will scroll to the cell with the same starting column value and typed string
+     * If the user presses any other special key as well, e.g. alt or shift we don't jump
      *
      * @param sortedColumn The sorted column in {@link MainTable}
      * @param keyEvent     The pressed character
      */
     private void jumpToSearchKey(TableColumn<BibEntryTableViewModel, ?> sortedColumn, KeyEvent keyEvent) {
+        if (keyEvent.isAltDown() || keyEvent.isControlDown() || keyEvent.isMetaDown() || keyEvent.isShiftDown()) {
+            return;
+        }
         if ((keyEvent.getCharacter() == null) || (sortedColumn == null)) {
             return;
         }
@@ -338,9 +342,8 @@ public void paste() {
         if (entriesToAdd.isEmpty()) {
             return;
         }
-        for (BibEntry entry : entriesToAdd) {
-            importHandler.importEntryWithDuplicateCheck(database, entry);
-        }
+
+        importHandler.importEntriesWithDuplicateCheck(database, entriesToAdd);
     }
 
     private List<BibEntry> handleNonBibTeXStringData(String data) {
@@ -359,9 +362,7 @@ private List<BibEntry> handleNonBibTeXStringData(String data) {
     }
 
     public void dropEntry(List<BibEntry> entriesToAdd) {
-        for (BibEntry entry : entriesToAdd) {
-            importHandler.importEntryWithDuplicateCheck(database, (BibEntry) entry.clone());
-        }
+        importHandler.importEntriesWithDuplicateCheck(database, entriesToAdd);
     }
 
     private void handleOnDragOver(TableRow<BibEntryTableViewModel> row, BibEntryTableViewModel item, DragEvent event) {
@@ -424,7 +425,8 @@ private void handleOnDragDropped(TableRow<BibEntryTableViewModel> row, BibEntryT
             // Center -> link files to entry
             // Depending on the pressed modifier, move/copy/link files to drop target
             switch (ControlHelper.getDroppingMouseLocation(row, event)) {
-                case TOP, BOTTOM -> importHandler.importFilesInBackground(files).executeWith(taskExecutor);
+                case TOP, BOTTOM ->
+                        importHandler.importFilesInBackground(files).executeWith(taskExecutor);
                 case CENTER -> {
                     BibEntry entry = target.getEntry();
                     switch (event.getTransferMode()) {

diff --git a/src/main/java/org/jabref/gui/mergeentries/MultiMergeEntriesView.java b/src/main/java/org/jabref/gui/mergeentries/MultiMergeEntriesView.java
@@ -144,8 +144,8 @@ private void fillDiffModes() {
         new ViewModelListCellFactory<DiffMode>()
                 .withText(DiffMode::getDisplayText)
                 .install(diffMode);
-        diffMode.setValue(preferences.getGuiPreferences().getMergeDiffMode());
-        EasyBind.subscribe(this.diffMode.valueProperty(), mode -> preferences.getGuiPreferences().setMergeDiffMode(mode));
+        diffMode.setValue(preferences.getMergeDialogPreferences().getMergeDiffMode());
+        EasyBind.subscribe(this.diffMode.valueProperty(), mode -> preferences.getMergeDialogPreferences().setMergeDiffMode(mode));
     }
 
     private void addColumn(MultiMergeEntriesViewModel.EntrySource entrySourceColumn) {

diff --git a/src/main/java/org/jabref/gui/mergeentries/newmergedialog/ThreeWayMergeHeaderView.java b/src/main/java/org/jabref/gui/mergeentries/newmergedialog/ThreeWayMergeHeaderView.java
@@ -23,7 +23,7 @@ public ThreeWayMergeHeaderView(String leftHeader, String rightHeader) {
 
         this.leftHeaderCell = new HeaderCell(leftHeader);
         this.rightHeaderCell = new HeaderCell(rightHeader);
-        this.mergedHeaderCell = new HeaderCell(Localization.lang("Merged Entry"));
+        this.mergedHeaderCell = new HeaderCell(Localization.lang("Merged entry"));
 
         addRow(0,
                new HeaderCell(""),

diff --git a/src/main/java/org/jabref/gui/mergeentries/newmergedialog/ThreeWayMergeViewModel.java b/src/main/java/org/jabref/gui/mergeentries/newmergedialog/ThreeWayMergeViewModel.java
@@ -111,6 +111,6 @@ private void setVisibleFields(Set<Field> fields) {
         visibleFields.sort(Comparator.comparing(Field::getName));
 
         // Add the entry type field as the first field to display
-        visibleFields.add(0, InternalField.TYPE_HEADER);
+        visibleFields.addFirst(InternalField.TYPE_HEADER);
     }
 }