From 400c6b0a1fd4be74d4e2bdac8359f5b79c704151 Mon Sep 17 00:00:00 2001 From: Alex Sukhin Date: Wed, 10 Sep 2025 23:53:02 +0100 Subject: [PATCH 01/27] Add cleanup dialog tabs with individual tab preferences --- CHANGELOG.md | 1 + .../org/jabref/gui/cleanup/CleanupAction.java | 314 +++++++++--------- .../org/jabref/gui/cleanup/CleanupDialog.java | 39 ++- .../gui/cleanup/CleanupFileRelatedPanel.java | 96 ++++++ .../gui/cleanup/CleanupMultiFieldPanel.java | 99 ++++++ .../org/jabref/gui/cleanup/CleanupPanel.java | 7 + .../gui/cleanup/CleanupPresetPanel.java | 173 ---------- .../gui/cleanup/CleanupSingleFieldPanel.java | 40 +++ .../org/jabref/gui/cleanup/CleanupDialog.fxml | 13 + .../gui/cleanup/CleanupFileRelatedPanel.fxml | 32 ++ .../gui/cleanup/CleanupMultiFieldPanel.fxml | 26 ++ .../gui/cleanup/CleanupPresetPanel.fxml | 52 --- .../gui/cleanup/CleanupSingleFieldPanel.fxml | 18 + .../logic/cleanup/CleanupPreferences.java | 81 +++-- .../logic/cleanup/CleanupPreferencesTest.java | 48 +++ 15 files changed, 625 insertions(+), 414 deletions(-) create mode 100644 jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java create mode 100644 jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java create mode 100644 jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java delete mode 100644 jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPresetPanel.java create mode 100644 jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java create mode 100644 jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupDialog.fxml create mode 100644 jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml create mode 100644 jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupMultiFieldPanel.fxml delete mode 100644 jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupPresetPanel.fxml create mode 100644 jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupSingleFieldPanel.fxml create mode 100644 jablib/src/test/java/org/jabref/logic/cleanup/CleanupPreferencesTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 72cd08f99a2..ef90a64f92c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We improved the event viewer for debugging [#13783](https://github.com/JabRef/jabref/pull/13783). - We improved "REDACTED" replacement of API key value in web fetcher search URL [#13796](https://github.com/JabRef/jabref/issues/13796) - When the pin "Keep dialog always on top" in the global search dialog is selected, the search window stays open when double-clicking on an entry. [#13840](https://github.com/JabRef/jabref/issues/13840) +- We separated the "Clean up entries" dialog into three tabs for clarity [13819](https://github.com/JabRef/jabref/issues/13819) ### Fixed diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java index 1a59a432559..5754cc65d23 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java @@ -1,182 +1,184 @@ -package org.jabref.gui.cleanup; - -import java.util.ArrayList; -import java.util.List; -import java.util.Optional; -import java.util.function.Supplier; -import java.util.stream.Collectors; - -import javax.swing.undo.UndoManager; - -import javafx.application.Platform; - -import org.jabref.gui.DialogService; -import org.jabref.gui.LibraryTab; -import org.jabref.gui.StateManager; -import org.jabref.gui.actions.ActionHelper; -import org.jabref.gui.actions.SimpleCommand; -import org.jabref.gui.undo.NamedCompound; -import org.jabref.gui.undo.UndoableFieldChange; -import org.jabref.logic.JabRefException; -import org.jabref.logic.cleanup.CleanupPreferences; -import org.jabref.logic.cleanup.CleanupWorker; -import org.jabref.logic.l10n.Localization; -import org.jabref.logic.preferences.CliPreferences; -import org.jabref.logic.util.BackgroundTask; -import org.jabref.logic.util.TaskExecutor; -import org.jabref.model.FieldChange; -import org.jabref.model.database.BibDatabaseContext; -import org.jabref.model.entry.BibEntry; - -public class CleanupAction extends SimpleCommand { - - private final Supplier tabSupplier; - private final CliPreferences preferences; - private final DialogService dialogService; - private final StateManager stateManager; - private final TaskExecutor taskExecutor; - private final UndoManager undoManager; - private final List failures; - - private boolean isCanceled; - private int modifiedEntriesCount; - - public CleanupAction(Supplier tabSupplier, - CliPreferences preferences, - DialogService dialogService, - StateManager stateManager, - TaskExecutor taskExecutor, - UndoManager undoManager) { - this.tabSupplier = tabSupplier; - this.preferences = preferences; - this.dialogService = dialogService; - this.stateManager = stateManager; - this.taskExecutor = taskExecutor; - this.undoManager = undoManager; - this.failures = new ArrayList<>(); - - this.executable.bind(ActionHelper.needsEntriesSelected(stateManager)); - } - - @Override - public void execute() { - if (stateManager.getActiveDatabase().isEmpty()) { - return; + package org.jabref.gui.cleanup; + + import java.util.ArrayList; + import java.util.List; + import java.util.Optional; + import java.util.function.Supplier; + import java.util.stream.Collectors; + + import javax.swing.undo.UndoManager; + + import javafx.application.Platform; + + import org.jabref.gui.DialogService; + import org.jabref.gui.LibraryTab; + import org.jabref.gui.StateManager; + import org.jabref.gui.actions.ActionHelper; + import org.jabref.gui.actions.SimpleCommand; + import org.jabref.gui.undo.NamedCompound; + import org.jabref.gui.undo.UndoableFieldChange; + import org.jabref.logic.JabRefException; + import org.jabref.logic.cleanup.CleanupPreferences; + import org.jabref.logic.cleanup.CleanupWorker; + import org.jabref.logic.l10n.Localization; + import org.jabref.logic.preferences.CliPreferences; + import org.jabref.logic.util.BackgroundTask; + import org.jabref.logic.util.TaskExecutor; + import org.jabref.model.FieldChange; + import org.jabref.model.database.BibDatabaseContext; + import org.jabref.model.entry.BibEntry; + + public class CleanupAction extends SimpleCommand { + + private final Supplier tabSupplier; + private final CliPreferences preferences; + private final DialogService dialogService; + private final StateManager stateManager; + private final TaskExecutor taskExecutor; + private final UndoManager undoManager; + private final List failures; + + private boolean isCanceled; + private int modifiedEntriesCount; + + public CleanupAction(Supplier tabSupplier, + CliPreferences preferences, + DialogService dialogService, + StateManager stateManager, + TaskExecutor taskExecutor, + UndoManager undoManager) { + this.tabSupplier = tabSupplier; + this.preferences = preferences; + this.dialogService = dialogService; + this.stateManager = stateManager; + this.taskExecutor = taskExecutor; + this.undoManager = undoManager; + this.failures = new ArrayList<>(); + + this.executable.bind(ActionHelper.needsEntriesSelected(stateManager)); } - if (stateManager.getSelectedEntries().isEmpty()) { // None selected. Inform the user to select entries first. - dialogService.showInformationDialogAndWait(Localization.lang("Cleanup entry"), Localization.lang("First select entries to clean up.")); - return; - } + @Override + public void execute() { + if (stateManager.getActiveDatabase().isEmpty()) { + return; + } - isCanceled = false; - modifiedEntriesCount = 0; - - CleanupDialog cleanupDialog = new CleanupDialog( - stateManager.getActiveDatabase().get(), - preferences.getCleanupPreferences(), - preferences.getFilePreferences() - ); - - Optional chosenPreset = dialogService.showCustomDialogAndWait(cleanupDialog); - - chosenPreset.ifPresent(preset -> { - if (preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF) && preferences.getAutoLinkPreferences().shouldAskAutoNamingPdfs()) { - boolean confirmed = dialogService.showConfirmationDialogWithOptOutAndWait(Localization.lang("Autogenerate PDF Names"), - Localization.lang("Auto-generating PDF-Names does not support undo. Continue?"), - Localization.lang("Autogenerate PDF Names"), - Localization.lang("Cancel"), - Localization.lang("Do not ask again"), - optOut -> preferences.getAutoLinkPreferences().setAskAutoNamingPdfs(!optOut)); - if (!confirmed) { - isCanceled = true; - return; - } + if (stateManager.getSelectedEntries().isEmpty()) { // None selected. Inform the user to select entries first. + dialogService.showInformationDialogAndWait(Localization.lang("Cleanup entry"), Localization.lang("First select entries to clean up.")); + return; } - preferences.getCleanupPreferences().setActiveJobs(preset.getActiveJobs()); - preferences.getCleanupPreferences().setFieldFormatterCleanups(preset.getFieldFormatterCleanups()); + isCanceled = false; + modifiedEntriesCount = 0; + + CleanupDialog cleanupDialog = new CleanupDialog( + stateManager.getActiveDatabase().get(), + preferences.getCleanupPreferences(), + preferences.getFilePreferences() + ); + + Optional chosenPreset = dialogService.showCustomDialogAndWait(cleanupDialog); + + chosenPreset.ifPresent(preset -> { + if (preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF) && preferences.getAutoLinkPreferences().shouldAskAutoNamingPdfs()) { + boolean confirmed = dialogService.showConfirmationDialogWithOptOutAndWait(Localization.lang("Autogenerate PDF Names"), + Localization.lang("Auto-generating PDF-Names does not support undo. Continue?"), + Localization.lang("Autogenerate PDF Names"), + Localization.lang("Cancel"), + Localization.lang("Do not ask again"), + optOut -> preferences.getAutoLinkPreferences().setAskAutoNamingPdfs(!optOut)); + if (!confirmed) { + isCanceled = true; + return; + } + } + + CleanupPreferences updatedPreferences = preferences.getCleanupPreferences().updateWith(preset); - BackgroundTask.wrap(() -> cleanup(stateManager.getActiveDatabase().get(), preset)) - .onSuccess(result -> showResults()) - .onFailure(dialogService::showErrorDialogAndWait) - .executeWith(taskExecutor); - }); - } + preferences.getCleanupPreferences().setActiveJobs(updatedPreferences.getActiveJobs()); + preferences.getCleanupPreferences().setFieldFormatterCleanups(updatedPreferences.getFieldFormatterCleanups()); - /** - * Runs the cleanup on the entry and records the change. - * - * @return true iff entry was modified - */ - private boolean doCleanup(BibDatabaseContext databaseContext, CleanupPreferences preset, BibEntry entry, NamedCompound ce) { - // Create and run cleaner - CleanupWorker cleaner = new CleanupWorker( - databaseContext, - preferences.getFilePreferences(), - preferences.getTimestampPreferences() - ); - - List changes = cleaner.cleanup(preset, entry); - - // Register undo action - for (FieldChange change : changes) { - ce.addEdit(new UndoableFieldChange(change)); + BackgroundTask.wrap(() -> cleanup(stateManager.getActiveDatabase().get(), preset)) + .onSuccess(result -> showResults()) + .onFailure(dialogService::showErrorDialogAndWait) + .executeWith(taskExecutor); + }); } - failures.addAll(cleaner.getFailures()); + /** + * Runs the cleanup on the entry and records the change. + * + * @return true iff entry was modified + */ + private boolean doCleanup(BibDatabaseContext databaseContext, CleanupPreferences preset, BibEntry entry, NamedCompound ce) { + // Create and run cleaner + CleanupWorker cleaner = new CleanupWorker( + databaseContext, + preferences.getFilePreferences(), + preferences.getTimestampPreferences() + ); + + List changes = cleaner.cleanup(preset, entry); + + // Register undo action + for (FieldChange change : changes) { + ce.addEdit(new UndoableFieldChange(change)); + } - return !changes.isEmpty(); - } + failures.addAll(cleaner.getFailures()); - private void showResults() { - if (isCanceled) { - return; + return !changes.isEmpty(); } - if (modifiedEntriesCount > 0) { - tabSupplier.get().markBaseChanged(); - } + private void showResults() { + if (isCanceled) { + return; + } - if (modifiedEntriesCount == 0) { - dialogService.notify(Localization.lang("No entry needed a clean up")); - } else if (modifiedEntriesCount == 1) { - dialogService.notify(Localization.lang("One entry needed a clean up")); - } else { - dialogService.notify(Localization.lang("%0 entries needed a clean up", Integer.toString(modifiedEntriesCount))); + if (modifiedEntriesCount > 0) { + tabSupplier.get().markBaseChanged(); + } + + if (modifiedEntriesCount == 0) { + dialogService.notify(Localization.lang("No entry needed a clean up")); + } else if (modifiedEntriesCount == 1) { + dialogService.notify(Localization.lang("One entry needed a clean up")); + } else { + dialogService.notify(Localization.lang("%0 entries needed a clean up", Integer.toString(modifiedEntriesCount))); + } } - } - private void cleanup(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences) { - this.failures.clear(); + private void cleanup(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences) { + this.failures.clear(); - // undo granularity is on set of all entries - NamedCompound ce = new NamedCompound(Localization.lang("Clean up entries")); + // undo granularity is on set of all entries + NamedCompound ce = new NamedCompound(Localization.lang("Clean up entries")); - for (BibEntry entry : List.copyOf(stateManager.getSelectedEntries())) { - if (doCleanup(databaseContext, cleanupPreferences, entry, ce)) { - modifiedEntriesCount++; + for (BibEntry entry : List.copyOf(stateManager.getSelectedEntries())) { + if (doCleanup(databaseContext, cleanupPreferences, entry, ce)) { + modifiedEntriesCount++; + } } - } - ce.end(); + ce.end(); - if (ce.hasEdits()) { - undoManager.addEdit(ce); - } + if (ce.hasEdits()) { + undoManager.addEdit(ce); + } - if (!failures.isEmpty()) { - showFailures(failures); + if (!failures.isEmpty()) { + showFailures(failures); + } } - } - private void showFailures(List failures) { - String message = failures.stream() - .map(exception -> "- " + exception.getLocalizedMessage()) - .collect(Collectors.joining("\n")); + private void showFailures(List failures) { + String message = failures.stream() + .map(exception -> "- " + exception.getLocalizedMessage()) + .collect(Collectors.joining("\n")); - Platform.runLater(() -> - dialogService.showErrorDialogAndWait(Localization.lang("File Move Errors"), message) - ); + Platform.runLater(() -> + dialogService.showErrorDialogAndWait(Localization.lang("File Move Errors"), message) + ); + } } -} diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java index 752b9309dc6..068c7a82cef 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java @@ -1,7 +1,9 @@ package org.jabref.gui.cleanup; -import javafx.scene.control.ButtonType; -import javafx.scene.control.ScrollPane; +import javafx.fxml.FXML; +import javafx.scene.control.ButtonBar; +import javafx.scene.control.Tab; +import javafx.scene.control.TabPane; import org.jabref.gui.util.BaseDialog; import org.jabref.logic.FilePreferences; @@ -9,24 +11,35 @@ import org.jabref.logic.l10n.Localization; import org.jabref.model.database.BibDatabaseContext; +import com.airhacks.afterburner.views.ViewLoader; + public class CleanupDialog extends BaseDialog { + + @FXML private TabPane tabPane; + public CleanupDialog(BibDatabaseContext databaseContext, CleanupPreferences initialPreset, FilePreferences filePreferences) { setTitle(Localization.lang("Clean up entries")); - getDialogPane().setPrefSize(600, 650); - getDialogPane().getButtonTypes().setAll(ButtonType.OK, ButtonType.CANCEL); - CleanupPresetPanel presetPanel = new CleanupPresetPanel(databaseContext, initialPreset, filePreferences); + // Load FXML + ViewLoader.view(this) + .load() + .setAsDialogPane(this); + + CleanupSingleFieldPanel singleFieldPanel = new CleanupSingleFieldPanel(initialPreset); + CleanupFileRelatedPanel fileRelatedPanel = new CleanupFileRelatedPanel(databaseContext, initialPreset, filePreferences); + CleanupMultiFieldPanel multiFieldPanel = new CleanupMultiFieldPanel(initialPreset); - // placing the content of the presetPanel in a scroll pane - ScrollPane scrollPane = new ScrollPane(); - scrollPane.setFitToWidth(true); - scrollPane.setFitToHeight(true); - scrollPane.setContent(presetPanel); + tabPane.getTabs().addAll( + new Tab(Localization.lang("Single field"), singleFieldPanel), + new Tab(Localization.lang("File-related"), fileRelatedPanel), + new Tab(Localization.lang("Multi-field"), multiFieldPanel) + ); - getDialogPane().setContent(scrollPane); setResultConverter(button -> { - if (button == ButtonType.OK) { - return presetPanel.getCleanupPreset(); + if (button.getButtonData() == ButtonBar.ButtonData.OK_DONE) { + Tab selectedTab = tabPane.getSelectionModel().getSelectedItem(); + CleanupPanel panel = (CleanupPanel) selectedTab.getContent(); + return panel.getCleanupPreferences(); } else { return null; } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java new file mode 100644 index 00000000000..e7b7674a869 --- /dev/null +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java @@ -0,0 +1,96 @@ +package org.jabref.gui.cleanup; + +import java.nio.file.Path; +import java.util.EnumSet; +import java.util.Optional; + +import javafx.fxml.FXML; +import javafx.scene.control.CheckBox; +import javafx.scene.control.Label; +import javafx.scene.layout.VBox; + +import org.jabref.logic.FilePreferences; +import org.jabref.logic.cleanup.CleanupPreferences; +import org.jabref.logic.l10n.Localization; +import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.field.StandardField; + +import com.airhacks.afterburner.views.ViewLoader; + +public class CleanupFileRelatedPanel extends VBox implements CleanupPanel { + + @FXML private Label cleanupRenamePDFLabel; + + @FXML private CheckBox cleanUpMovePDF; + @FXML private CheckBox cleanUpMakePathsRelative; + @FXML private CheckBox cleanUpRenamePDF; + @FXML private CheckBox cleanUpRenamePDFonlyRelativePaths; + @FXML private CheckBox cleanUpDeletedFiles; + @FXML private CheckBox cleanUpUpgradeExternalLinks; + + public CleanupFileRelatedPanel(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences, FilePreferences filePreferences) { + // Load FXML + ViewLoader.view(this) + .root(this) + .load(); + + init(databaseContext, cleanupPreferences, filePreferences); + } + + private void init(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences, FilePreferences filePreferences) { + Optional firstExistingDir = databaseContext.getFirstExistingFileDir(filePreferences); + if (firstExistingDir.isPresent()) { + cleanUpMovePDF.setText(Localization.lang("Move linked files to default file directory %0", firstExistingDir.get().toString())); + } else { + cleanUpMovePDF.setText(Localization.lang("Move linked files to default file directory %0", "...")); + + // Since the directory does not exist, we cannot move it to there. So, this option is not checked - regardless of the presets stored in the preferences. + cleanUpMovePDF.setDisable(true); + cleanUpMovePDF.setSelected(false); + } + + cleanUpRenamePDFonlyRelativePaths.disableProperty().bind(cleanUpRenamePDF.selectedProperty().not()); + + cleanUpUpgradeExternalLinks.setText(Localization.lang("Upgrade external PDF/PS links to use the '%0' field.", StandardField.FILE.getDisplayName())); + + String currentPattern = Localization.lang("Filename format pattern (from preferences)") + .concat(filePreferences.getFileNamePattern()); + cleanupRenamePDFLabel.setText(currentPattern); + + updateDisplay(cleanupPreferences); + } + + private void updateDisplay(CleanupPreferences preset) { + if (!cleanUpMovePDF.isDisabled()) { + cleanUpMovePDF.setSelected(preset.isActive(CleanupPreferences.CleanupStep.MOVE_PDF)); + } + cleanUpMakePathsRelative.setSelected(preset.isActive(CleanupPreferences.CleanupStep.MAKE_PATHS_RELATIVE)); + cleanUpRenamePDF.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF)); + cleanUpRenamePDFonlyRelativePaths.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS)); + cleanUpUpgradeExternalLinks.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_UPGRADE_EXTERNAL_LINKS)); + cleanUpDeletedFiles.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES)); + } + + public CleanupPreferences getCleanupPreferences() { + EnumSet activeJobs = EnumSet.noneOf(CleanupPreferences.CleanupStep.class); + + if (cleanUpMakePathsRelative.isSelected()) { + activeJobs.add(CleanupPreferences.CleanupStep.MAKE_PATHS_RELATIVE); + } + if (cleanUpRenamePDF.isSelected()) { + if (cleanUpRenamePDFonlyRelativePaths.isSelected()) { + activeJobs.add(CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS); + } else { + activeJobs.add(CleanupPreferences.CleanupStep.RENAME_PDF); + } + } + if (cleanUpUpgradeExternalLinks.isSelected()) { + activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_UPGRADE_EXTERNAL_LINKS); + } + if (cleanUpDeletedFiles.isSelected()) { + activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES); + } + + return new CleanupPreferences(activeJobs); + } +} diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java new file mode 100644 index 00000000000..f4b23c61ffd --- /dev/null +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java @@ -0,0 +1,99 @@ +package org.jabref.gui.cleanup; + +import java.util.EnumSet; + +import javafx.fxml.FXML; +import javafx.scene.control.CheckBox; +import javafx.scene.layout.VBox; + +import org.jabref.logic.cleanup.CleanupPreferences; + +import com.airhacks.afterburner.views.ViewLoader; + +public class CleanupMultiFieldPanel extends VBox implements CleanupPanel { + @FXML private CheckBox cleanUpDOI; + @FXML private CheckBox cleanUpEprint; + @FXML private CheckBox cleanUpURL; + @FXML private CheckBox cleanUpBiblatex; + @FXML private CheckBox cleanUpBibtex; + @FXML private CheckBox cleanUpTimestampToCreationDate; + @FXML private CheckBox cleanUpTimestampToModificationDate; + + public CleanupMultiFieldPanel(CleanupPreferences cleanupPreferences) { + // Load FXML + ViewLoader.view(this) + .root(this) + .load(); + + init(cleanupPreferences); + } + + private void init(CleanupPreferences cleanupPreferences) { + cleanUpBibtex.selectedProperty().addListener( + (observable, oldValue, newValue) -> { + if (newValue) { + cleanUpBiblatex.selectedProperty().setValue(false); + } + }); + cleanUpBiblatex.selectedProperty().addListener( + (observable, oldValue, newValue) -> { + if (newValue) { + cleanUpBibtex.selectedProperty().setValue(false); + } + }); + + cleanUpTimestampToCreationDate.selectedProperty().addListener( + (observable, oldValue, newValue) -> { + if (newValue) { + cleanUpTimestampToModificationDate.selectedProperty().setValue(false); + } + }); + cleanUpTimestampToModificationDate.selectedProperty().addListener( + (observable, oldValue, newValue) -> { + if (newValue) { + cleanUpTimestampToCreationDate.selectedProperty().setValue(false); + } + }); + updateDisplay(cleanupPreferences); + } + + private void updateDisplay(CleanupPreferences preset) { + cleanUpDOI.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_DOI)); + cleanUpEprint.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEANUP_EPRINT)); + cleanUpURL.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_URL)); + cleanUpBiblatex.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CONVERT_TO_BIBLATEX)); + cleanUpBibtex.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CONVERT_TO_BIBTEX)); + cleanUpTimestampToCreationDate.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_CREATIONDATE)); + cleanUpTimestampToModificationDate.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_MODIFICATIONDATE)); + cleanUpTimestampToModificationDate.setSelected(preset.isActive(CleanupPreferences.CleanupStep.DO_NOT_CONVERT_TIMESTAMP)); + } + + @Override + public CleanupPreferences getCleanupPreferences() { + EnumSet activeJobs = EnumSet.noneOf(CleanupPreferences.CleanupStep.class); + + if (cleanUpDOI.isSelected()) { + activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_DOI); + } + if (cleanUpEprint.isSelected()) { + activeJobs.add(CleanupPreferences.CleanupStep.CLEANUP_EPRINT); + } + if (cleanUpURL.isSelected()) { + activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_URL); + } + if (cleanUpBiblatex.isSelected()) { + activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TO_BIBLATEX); + } + if (cleanUpBibtex.isSelected()) { + activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TO_BIBTEX); + } + if (cleanUpTimestampToCreationDate.isSelected()) { + activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_CREATIONDATE); + } + if (cleanUpTimestampToModificationDate.isSelected()) { + activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_MODIFICATIONDATE); + } + + return new CleanupPreferences(activeJobs); + } +} diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java new file mode 100644 index 00000000000..0d64e172a2e --- /dev/null +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java @@ -0,0 +1,7 @@ +package org.jabref.gui.cleanup; + +import org.jabref.logic.cleanup.CleanupPreferences; + +public interface CleanupPanel { + CleanupPreferences getCleanupPreferences(); +} diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPresetPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPresetPanel.java deleted file mode 100644 index b0bbc74dc6d..00000000000 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPresetPanel.java +++ /dev/null @@ -1,173 +0,0 @@ -package org.jabref.gui.cleanup; - -import java.nio.file.Path; -import java.util.EnumSet; -import java.util.Objects; -import java.util.Optional; - -import javafx.collections.FXCollections; -import javafx.fxml.FXML; -import javafx.scene.control.CheckBox; -import javafx.scene.control.Label; -import javafx.scene.layout.VBox; - -import org.jabref.gui.commonfxcontrols.FieldFormatterCleanupsPanel; -import org.jabref.logic.FilePreferences; -import org.jabref.logic.cleanup.CleanupPreferences; -import org.jabref.logic.cleanup.FieldFormatterCleanups; -import org.jabref.logic.l10n.Localization; -import org.jabref.model.database.BibDatabaseContext; -import org.jabref.model.entry.field.StandardField; - -import com.airhacks.afterburner.views.ViewLoader; - -public class CleanupPresetPanel extends VBox { - - private final BibDatabaseContext databaseContext; - @FXML private Label cleanupRenamePDFLabel; - @FXML private CheckBox cleanUpDOI; - @FXML private CheckBox cleanUpEprint; - @FXML private CheckBox cleanUpURL; - @FXML private CheckBox cleanUpMovePDF; - @FXML private CheckBox cleanUpMakePathsRelative; - @FXML private CheckBox cleanUpRenamePDF; - @FXML private CheckBox cleanUpRenamePDFonlyRelativePaths; - @FXML private CheckBox cleanUpDeletedFiles; - @FXML private CheckBox cleanUpUpgradeExternalLinks; - @FXML private CheckBox cleanUpBiblatex; - @FXML private CheckBox cleanUpBibtex; - @FXML private CheckBox cleanUpTimestampToCreationDate; - @FXML private CheckBox cleanUpTimestampToModificationDate; - @FXML private FieldFormatterCleanupsPanel formatterCleanupsPanel; - - public CleanupPresetPanel(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences, FilePreferences filePreferences) { - this.databaseContext = Objects.requireNonNull(databaseContext); - - // Load FXML - ViewLoader.view(this) - .root(this) - .load(); - - init(cleanupPreferences, filePreferences); - } - - private void init(CleanupPreferences cleanupPreferences, FilePreferences filePreferences) { - Optional firstExistingDir = databaseContext.getFirstExistingFileDir(filePreferences); - if (firstExistingDir.isPresent()) { - cleanUpMovePDF.setText(Localization.lang("Move linked files to default file directory %0", firstExistingDir.get().toString())); - } else { - cleanUpMovePDF.setText(Localization.lang("Move linked files to default file directory %0", "...")); - - // Since the directory does not exist, we cannot move it to there. So, this option is not checked - regardless of the presets stored in the preferences. - cleanUpMovePDF.setDisable(true); - cleanUpMovePDF.setSelected(false); - } - - cleanUpRenamePDFonlyRelativePaths.disableProperty().bind(cleanUpRenamePDF.selectedProperty().not()); - - cleanUpUpgradeExternalLinks.setText(Localization.lang("Upgrade external PDF/PS links to use the '%0' field.", StandardField.FILE.getDisplayName())); - - String currentPattern = Localization.lang("Filename format pattern (from preferences)") - .concat(filePreferences.getFileNamePattern()); - cleanupRenamePDFLabel.setText(currentPattern); - - cleanUpBibtex.selectedProperty().addListener( - (observable, oldValue, newValue) -> { - if (newValue) { - cleanUpBiblatex.selectedProperty().setValue(false); - } - }); - cleanUpBiblatex.selectedProperty().addListener( - (observable, oldValue, newValue) -> { - if (newValue) { - cleanUpBibtex.selectedProperty().setValue(false); - } - }); - - cleanUpTimestampToCreationDate.selectedProperty().addListener( - (observable, oldValue, newValue) -> { - if (newValue) { - cleanUpTimestampToModificationDate.selectedProperty().setValue(false); - } - }); - cleanUpTimestampToModificationDate.selectedProperty().addListener( - (observable, oldValue, newValue) -> { - if (newValue) { - cleanUpTimestampToCreationDate.selectedProperty().setValue(false); - } - }); - updateDisplay(cleanupPreferences); - } - - private void updateDisplay(CleanupPreferences preset) { - cleanUpDOI.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_DOI)); - cleanUpEprint.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEANUP_EPRINT)); - cleanUpURL.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_URL)); - if (!cleanUpMovePDF.isDisabled()) { - cleanUpMovePDF.setSelected(preset.isActive(CleanupPreferences.CleanupStep.MOVE_PDF)); - } - cleanUpMakePathsRelative.setSelected(preset.isActive(CleanupPreferences.CleanupStep.MAKE_PATHS_RELATIVE)); - cleanUpRenamePDF.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF)); - cleanUpRenamePDFonlyRelativePaths.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS)); - cleanUpUpgradeExternalLinks.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_UPGRADE_EXTERNAL_LINKS)); - cleanUpDeletedFiles.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES)); - cleanUpBiblatex.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CONVERT_TO_BIBLATEX)); - cleanUpBibtex.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CONVERT_TO_BIBTEX)); - cleanUpTimestampToCreationDate.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_CREATIONDATE)); - cleanUpTimestampToModificationDate.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_MODIFICATIONDATE)); - cleanUpTimestampToModificationDate.setSelected(preset.isActive(CleanupPreferences.CleanupStep.DO_NOT_CONVERT_TIMESTAMP)); - formatterCleanupsPanel.cleanupsDisableProperty().setValue(!preset.getFieldFormatterCleanups().isEnabled()); - formatterCleanupsPanel.cleanupsProperty().setValue(FXCollections.observableArrayList(preset.getFieldFormatterCleanups().getConfiguredActions())); - } - - public CleanupPreferences getCleanupPreset() { - EnumSet activeJobs = EnumSet.noneOf(CleanupPreferences.CleanupStep.class); - - if (cleanUpMovePDF.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.MOVE_PDF); - } - if (cleanUpDOI.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_DOI); - } - if (cleanUpEprint.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.CLEANUP_EPRINT); - } - if (cleanUpURL.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_URL); - } - if (cleanUpMakePathsRelative.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.MAKE_PATHS_RELATIVE); - } - if (cleanUpRenamePDF.isSelected()) { - if (cleanUpRenamePDFonlyRelativePaths.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS); - } else { - activeJobs.add(CleanupPreferences.CleanupStep.RENAME_PDF); - } - } - if (cleanUpUpgradeExternalLinks.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_UPGRADE_EXTERNAL_LINKS); - } - if (cleanUpDeletedFiles.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES); - } - if (cleanUpBiblatex.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TO_BIBLATEX); - } - if (cleanUpBibtex.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TO_BIBTEX); - } - if (cleanUpTimestampToCreationDate.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_CREATIONDATE); - } - if (cleanUpTimestampToModificationDate.isSelected()) { - activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_MODIFICATIONDATE); - } - - activeJobs.add(CleanupPreferences.CleanupStep.FIX_FILE_LINKS); - - return new CleanupPreferences(activeJobs, new FieldFormatterCleanups( - !formatterCleanupsPanel.cleanupsDisableProperty().getValue(), - formatterCleanupsPanel.cleanupsProperty())); - } -} diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java new file mode 100644 index 00000000000..da3e3d7b9ab --- /dev/null +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java @@ -0,0 +1,40 @@ +package org.jabref.gui.cleanup; + +import javafx.collections.FXCollections; +import javafx.fxml.FXML; +import javafx.scene.layout.VBox; + +import org.jabref.gui.commonfxcontrols.FieldFormatterCleanupsPanel; +import org.jabref.logic.cleanup.CleanupPreferences; +import org.jabref.logic.cleanup.FieldFormatterCleanups; + +import com.airhacks.afterburner.views.ViewLoader; + +public class CleanupSingleFieldPanel extends VBox implements CleanupPanel { + + @FXML private FieldFormatterCleanupsPanel formatterCleanupsPanel; + + public CleanupSingleFieldPanel(CleanupPreferences cleanupPreferences) { + ViewLoader.view(this) + .root(this) + .load(); + + init(cleanupPreferences); + } + + private void init(CleanupPreferences cleanupPreferences) { + formatterCleanupsPanel.cleanupsDisableProperty().setValue(!cleanupPreferences.getFieldFormatterCleanups().isEnabled()); + formatterCleanupsPanel.cleanupsProperty().setValue(FXCollections.observableArrayList( + cleanupPreferences.getFieldFormatterCleanups().getConfiguredActions() + )); + } + + @Override + public CleanupPreferences getCleanupPreferences() { + FieldFormatterCleanups fieldFormatterCleanups = new FieldFormatterCleanups( + !formatterCleanupsPanel.cleanupsDisableProperty().getValue(), + formatterCleanupsPanel.cleanupsProperty() + ); + return new CleanupPreferences(fieldFormatterCleanups); + } +} diff --git a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupDialog.fxml b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupDialog.fxml new file mode 100644 index 00000000000..85ee169dab3 --- /dev/null +++ b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupDialog.fxml @@ -0,0 +1,13 @@ + + + + + + + + + + + diff --git a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml new file mode 100644 index 00000000000..49af70a6003 --- /dev/null +++ b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml @@ -0,0 +1,32 @@ + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupMultiFieldPanel.fxml b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupMultiFieldPanel.fxml new file mode 100644 index 00000000000..a6bf6f1adad --- /dev/null +++ b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupMultiFieldPanel.fxml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupPresetPanel.fxml b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupPresetPanel.fxml deleted file mode 100644 index f6421d75a4d..00000000000 --- a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupPresetPanel.fxml +++ /dev/null @@ -1,52 +0,0 @@ - - - - - - - - - - - - - - diff --git a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupSingleFieldPanel.fxml b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupSingleFieldPanel.fxml new file mode 100644 index 00000000000..6c375e2684d --- /dev/null +++ b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupSingleFieldPanel.fxml @@ -0,0 +1,18 @@ + + + + + + + + + + + + + + + + diff --git a/jablib/src/main/java/org/jabref/logic/cleanup/CleanupPreferences.java b/jablib/src/main/java/org/jabref/logic/cleanup/CleanupPreferences.java index 61a04b9c455..a1117ec5ec9 100644 --- a/jablib/src/main/java/org/jabref/logic/cleanup/CleanupPreferences.java +++ b/jablib/src/main/java/org/jabref/logic/cleanup/CleanupPreferences.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.EnumSet; +import java.util.Optional; import java.util.Set; import javafx.beans.property.ObjectProperty; @@ -72,38 +73,78 @@ public void setFieldFormatterCleanups(FieldFormatterCleanups fieldFormatters) { fieldFormatterCleanups.setValue(fieldFormatters); } + public CleanupPreferences updateWith(CleanupPreferences tabPreferences) { + EnumSet mergedJobs = getActiveJobs(); + + Optional updatedCategory = + tabPreferences.getActiveJobs().stream() + .map(CleanupStep::getCategory) + .findFirst(); + + updatedCategory.ifPresent(category -> { + mergedJobs.removeIf(step -> step.getCategory() == category); + mergedJobs.addAll(tabPreferences.getActiveJobs()); + } + ); + + return new CleanupPreferences( + mergedJobs, + tabPreferences.getFieldFormatterCleanups() != null + ? tabPreferences.getFieldFormatterCleanups() + : getFieldFormatterCleanups() + ); + } + public enum CleanupStep { /** * Removes the http://... for each DOI. Moves DOIs from URL and NOTE filed to DOI field. */ - CLEAN_UP_DOI, - CLEANUP_EPRINT, - CLEAN_UP_URL, - MAKE_PATHS_RELATIVE, - RENAME_PDF, - RENAME_PDF_ONLY_RELATIVE_PATHS, - /** - * Collects file links from the pdf or ps field, and adds them to the list contained in the file field. - */ - CLEAN_UP_UPGRADE_EXTERNAL_LINKS, - CLEAN_UP_DELETED_LINKED_FILES, + CLEAN_UP_DOI(CleanupStepCategory.MULTI_FIELD), + CLEANUP_EPRINT(CleanupStepCategory.MULTI_FIELD), + CLEAN_UP_URL(CleanupStepCategory.MULTI_FIELD), /** * Converts to biblatex format */ - CONVERT_TO_BIBLATEX, + CONVERT_TO_BIBLATEX(CleanupStepCategory.MULTI_FIELD), /** * Converts to bibtex format */ - CONVERT_TO_BIBTEX, - CONVERT_TIMESTAMP_TO_CREATIONDATE, - CONVERT_TIMESTAMP_TO_MODIFICATIONDATE, - DO_NOT_CONVERT_TIMESTAMP, - MOVE_PDF, - FIX_FILE_LINKS, - CLEAN_UP_ISSN, + CONVERT_TO_BIBTEX(CleanupStepCategory.MULTI_FIELD), + CONVERT_TIMESTAMP_TO_CREATIONDATE(CleanupStepCategory.MULTI_FIELD), + CONVERT_TIMESTAMP_TO_MODIFICATIONDATE(CleanupStepCategory.MULTI_FIELD), + DO_NOT_CONVERT_TIMESTAMP(CleanupStepCategory.MULTI_FIELD), + + MOVE_PDF(CleanupStepCategory.FILE_RELATED), + MAKE_PATHS_RELATIVE(CleanupStepCategory.FILE_RELATED), + RENAME_PDF(CleanupStepCategory.FILE_RELATED), + RENAME_PDF_ONLY_RELATIVE_PATHS(CleanupStepCategory.FILE_RELATED), + /** + * Collects file links from the pdf or ps field, and adds them to the list contained in the file field. + */ + CLEAN_UP_UPGRADE_EXTERNAL_LINKS(CleanupStepCategory.FILE_RELATED), + CLEAN_UP_DELETED_LINKED_FILES(CleanupStepCategory.FILE_RELATED), + + FIX_FILE_LINKS(CleanupStepCategory.NONE), + CLEAN_UP_ISSN(CleanupStepCategory.NONE), /* * Converts Math Subject Classification Codes presented in Keywords into their Descriptions */ - CONVERT_MSC_CODES + CONVERT_MSC_CODES(CleanupStepCategory.NONE); + + private final CleanupStepCategory category; + + CleanupStep(CleanupStepCategory category) { + this.category = category; + } + + public CleanupStepCategory getCategory() { + return category; + } + } + + public enum CleanupStepCategory { + MULTI_FIELD, + FILE_RELATED, + NONE } } diff --git a/jablib/src/test/java/org/jabref/logic/cleanup/CleanupPreferencesTest.java b/jablib/src/test/java/org/jabref/logic/cleanup/CleanupPreferencesTest.java new file mode 100644 index 00000000000..a6beda55e48 --- /dev/null +++ b/jablib/src/test/java/org/jabref/logic/cleanup/CleanupPreferencesTest.java @@ -0,0 +1,48 @@ +package org.jabref.logic.cleanup; + +import java.util.EnumSet; +import java.util.Set; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class CleanupPreferencesTest { + + private CleanupPreferences cleanupPreferences; + + @BeforeEach + void setUp() { + cleanupPreferences = new CleanupPreferences( + EnumSet.of(CleanupPreferences.CleanupStep.CLEAN_UP_DOI) + ); + } + + @Test + void updateWithReplacesStepsInSameCategory() { + CleanupPreferences update = new CleanupPreferences( + EnumSet.of(CleanupPreferences.CleanupStep.CLEANUP_EPRINT) + ); + + CleanupPreferences result = cleanupPreferences.updateWith(update); + Set activeJobs = result.getActiveJobs(); + + assertFalse(activeJobs.contains(CleanupPreferences.CleanupStep.CLEAN_UP_DOI)); + assertTrue(activeJobs.contains(CleanupPreferences.CleanupStep.CLEANUP_EPRINT)); + } + + @Test + void updateWithDifferentCategoryKeepsOtherJobs() { + CleanupPreferences update = new CleanupPreferences( + EnumSet.of(CleanupPreferences.CleanupStep.MOVE_PDF) + ); + + CleanupPreferences result = cleanupPreferences.updateWith(update); + Set activeJobs = result.getActiveJobs(); + + assertTrue(activeJobs.contains(CleanupPreferences.CleanupStep.CLEAN_UP_DOI)); + assertTrue(activeJobs.contains(CleanupPreferences.CleanupStep.MOVE_PDF)); + } +} From dd2f74cfe3765e8a671c0bc7d928918dd1c0d60f Mon Sep 17 00:00:00 2001 From: Alex Sukhin Date: Thu, 11 Sep 2025 00:09:56 +0100 Subject: [PATCH 02/27] Fixed indentation and added commenting --- .../org/jabref/gui/cleanup/CleanupAction.java | 314 +++++++++--------- .../logic/cleanup/CleanupPreferences.java | 5 + 2 files changed, 162 insertions(+), 157 deletions(-) diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java index 5754cc65d23..0149a040dc6 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java @@ -1,184 +1,184 @@ - package org.jabref.gui.cleanup; - - import java.util.ArrayList; - import java.util.List; - import java.util.Optional; - import java.util.function.Supplier; - import java.util.stream.Collectors; - - import javax.swing.undo.UndoManager; - - import javafx.application.Platform; - - import org.jabref.gui.DialogService; - import org.jabref.gui.LibraryTab; - import org.jabref.gui.StateManager; - import org.jabref.gui.actions.ActionHelper; - import org.jabref.gui.actions.SimpleCommand; - import org.jabref.gui.undo.NamedCompound; - import org.jabref.gui.undo.UndoableFieldChange; - import org.jabref.logic.JabRefException; - import org.jabref.logic.cleanup.CleanupPreferences; - import org.jabref.logic.cleanup.CleanupWorker; - import org.jabref.logic.l10n.Localization; - import org.jabref.logic.preferences.CliPreferences; - import org.jabref.logic.util.BackgroundTask; - import org.jabref.logic.util.TaskExecutor; - import org.jabref.model.FieldChange; - import org.jabref.model.database.BibDatabaseContext; - import org.jabref.model.entry.BibEntry; - - public class CleanupAction extends SimpleCommand { - - private final Supplier tabSupplier; - private final CliPreferences preferences; - private final DialogService dialogService; - private final StateManager stateManager; - private final TaskExecutor taskExecutor; - private final UndoManager undoManager; - private final List failures; - - private boolean isCanceled; - private int modifiedEntriesCount; - - public CleanupAction(Supplier tabSupplier, - CliPreferences preferences, - DialogService dialogService, - StateManager stateManager, - TaskExecutor taskExecutor, - UndoManager undoManager) { - this.tabSupplier = tabSupplier; - this.preferences = preferences; - this.dialogService = dialogService; - this.stateManager = stateManager; - this.taskExecutor = taskExecutor; - this.undoManager = undoManager; - this.failures = new ArrayList<>(); - - this.executable.bind(ActionHelper.needsEntriesSelected(stateManager)); +package org.jabref.gui.cleanup; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +import javax.swing.undo.UndoManager; + +import javafx.application.Platform; + +import org.jabref.gui.DialogService; +import org.jabref.gui.LibraryTab; +import org.jabref.gui.StateManager; +import org.jabref.gui.actions.ActionHelper; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.undo.NamedCompound; +import org.jabref.gui.undo.UndoableFieldChange; +import org.jabref.logic.JabRefException; +import org.jabref.logic.cleanup.CleanupPreferences; +import org.jabref.logic.cleanup.CleanupWorker; +import org.jabref.logic.l10n.Localization; +import org.jabref.logic.preferences.CliPreferences; +import org.jabref.logic.util.BackgroundTask; +import org.jabref.logic.util.TaskExecutor; +import org.jabref.model.FieldChange; +import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.BibEntry; + +public class CleanupAction extends SimpleCommand { + + private final Supplier tabSupplier; + private final CliPreferences preferences; + private final DialogService dialogService; + private final StateManager stateManager; + private final TaskExecutor taskExecutor; + private final UndoManager undoManager; + private final List failures; + + private boolean isCanceled; + private int modifiedEntriesCount; + + public CleanupAction(Supplier tabSupplier, + CliPreferences preferences, + DialogService dialogService, + StateManager stateManager, + TaskExecutor taskExecutor, + UndoManager undoManager) { + this.tabSupplier = tabSupplier; + this.preferences = preferences; + this.dialogService = dialogService; + this.stateManager = stateManager; + this.taskExecutor = taskExecutor; + this.undoManager = undoManager; + this.failures = new ArrayList<>(); + + this.executable.bind(ActionHelper.needsEntriesSelected(stateManager)); + } + + @Override + public void execute() { + if (stateManager.getActiveDatabase().isEmpty()) { + return; } - @Override - public void execute() { - if (stateManager.getActiveDatabase().isEmpty()) { - return; - } + if (stateManager.getSelectedEntries().isEmpty()) { // None selected. Inform the user to select entries first. + dialogService.showInformationDialogAndWait(Localization.lang("Cleanup entry"), Localization.lang("First select entries to clean up.")); + return; + } - if (stateManager.getSelectedEntries().isEmpty()) { // None selected. Inform the user to select entries first. - dialogService.showInformationDialogAndWait(Localization.lang("Cleanup entry"), Localization.lang("First select entries to clean up.")); - return; + isCanceled = false; + modifiedEntriesCount = 0; + + CleanupDialog cleanupDialog = new CleanupDialog( + stateManager.getActiveDatabase().get(), + preferences.getCleanupPreferences(), + preferences.getFilePreferences() + ); + + Optional chosenPreset = dialogService.showCustomDialogAndWait(cleanupDialog); + + chosenPreset.ifPresent(preset -> { + if (preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF) && preferences.getAutoLinkPreferences().shouldAskAutoNamingPdfs()) { + boolean confirmed = dialogService.showConfirmationDialogWithOptOutAndWait(Localization.lang("Autogenerate PDF Names"), + Localization.lang("Auto-generating PDF-Names does not support undo. Continue?"), + Localization.lang("Autogenerate PDF Names"), + Localization.lang("Cancel"), + Localization.lang("Do not ask again"), + optOut -> preferences.getAutoLinkPreferences().setAskAutoNamingPdfs(!optOut)); + if (!confirmed) { + isCanceled = true; + return; + } } - isCanceled = false; - modifiedEntriesCount = 0; - - CleanupDialog cleanupDialog = new CleanupDialog( - stateManager.getActiveDatabase().get(), - preferences.getCleanupPreferences(), - preferences.getFilePreferences() - ); - - Optional chosenPreset = dialogService.showCustomDialogAndWait(cleanupDialog); - - chosenPreset.ifPresent(preset -> { - if (preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF) && preferences.getAutoLinkPreferences().shouldAskAutoNamingPdfs()) { - boolean confirmed = dialogService.showConfirmationDialogWithOptOutAndWait(Localization.lang("Autogenerate PDF Names"), - Localization.lang("Auto-generating PDF-Names does not support undo. Continue?"), - Localization.lang("Autogenerate PDF Names"), - Localization.lang("Cancel"), - Localization.lang("Do not ask again"), - optOut -> preferences.getAutoLinkPreferences().setAskAutoNamingPdfs(!optOut)); - if (!confirmed) { - isCanceled = true; - return; - } - } + CleanupPreferences updatedPreferences = preferences.getCleanupPreferences().updateWith(preset); - CleanupPreferences updatedPreferences = preferences.getCleanupPreferences().updateWith(preset); + preferences.getCleanupPreferences().setActiveJobs(updatedPreferences.getActiveJobs()); + preferences.getCleanupPreferences().setFieldFormatterCleanups(updatedPreferences.getFieldFormatterCleanups()); - preferences.getCleanupPreferences().setActiveJobs(updatedPreferences.getActiveJobs()); - preferences.getCleanupPreferences().setFieldFormatterCleanups(updatedPreferences.getFieldFormatterCleanups()); + BackgroundTask.wrap(() -> cleanup(stateManager.getActiveDatabase().get(), preset)) + .onSuccess(result -> showResults()) + .onFailure(dialogService::showErrorDialogAndWait) + .executeWith(taskExecutor); + }); + } - BackgroundTask.wrap(() -> cleanup(stateManager.getActiveDatabase().get(), preset)) - .onSuccess(result -> showResults()) - .onFailure(dialogService::showErrorDialogAndWait) - .executeWith(taskExecutor); - }); + /** + * Runs the cleanup on the entry and records the change. + * + * @return true iff entry was modified + */ + private boolean doCleanup(BibDatabaseContext databaseContext, CleanupPreferences preset, BibEntry entry, NamedCompound ce) { + // Create and run cleaner + CleanupWorker cleaner = new CleanupWorker( + databaseContext, + preferences.getFilePreferences(), + preferences.getTimestampPreferences() + ); + + List changes = cleaner.cleanup(preset, entry); + + // Register undo action + for (FieldChange change : changes) { + ce.addEdit(new UndoableFieldChange(change)); } - /** - * Runs the cleanup on the entry and records the change. - * - * @return true iff entry was modified - */ - private boolean doCleanup(BibDatabaseContext databaseContext, CleanupPreferences preset, BibEntry entry, NamedCompound ce) { - // Create and run cleaner - CleanupWorker cleaner = new CleanupWorker( - databaseContext, - preferences.getFilePreferences(), - preferences.getTimestampPreferences() - ); - - List changes = cleaner.cleanup(preset, entry); - - // Register undo action - for (FieldChange change : changes) { - ce.addEdit(new UndoableFieldChange(change)); - } + failures.addAll(cleaner.getFailures()); - failures.addAll(cleaner.getFailures()); + return !changes.isEmpty(); + } - return !changes.isEmpty(); + private void showResults() { + if (isCanceled) { + return; } - private void showResults() { - if (isCanceled) { - return; - } - - if (modifiedEntriesCount > 0) { - tabSupplier.get().markBaseChanged(); - } + if (modifiedEntriesCount > 0) { + tabSupplier.get().markBaseChanged(); + } - if (modifiedEntriesCount == 0) { - dialogService.notify(Localization.lang("No entry needed a clean up")); - } else if (modifiedEntriesCount == 1) { - dialogService.notify(Localization.lang("One entry needed a clean up")); - } else { - dialogService.notify(Localization.lang("%0 entries needed a clean up", Integer.toString(modifiedEntriesCount))); - } + if (modifiedEntriesCount == 0) { + dialogService.notify(Localization.lang("No entry needed a clean up")); + } else if (modifiedEntriesCount == 1) { + dialogService.notify(Localization.lang("One entry needed a clean up")); + } else { + dialogService.notify(Localization.lang("%0 entries needed a clean up", Integer.toString(modifiedEntriesCount))); } + } - private void cleanup(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences) { - this.failures.clear(); + private void cleanup(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences) { + this.failures.clear(); - // undo granularity is on set of all entries - NamedCompound ce = new NamedCompound(Localization.lang("Clean up entries")); + // undo granularity is on set of all entries + NamedCompound ce = new NamedCompound(Localization.lang("Clean up entries")); - for (BibEntry entry : List.copyOf(stateManager.getSelectedEntries())) { - if (doCleanup(databaseContext, cleanupPreferences, entry, ce)) { - modifiedEntriesCount++; - } + for (BibEntry entry : List.copyOf(stateManager.getSelectedEntries())) { + if (doCleanup(databaseContext, cleanupPreferences, entry, ce)) { + modifiedEntriesCount++; } + } - ce.end(); + ce.end(); - if (ce.hasEdits()) { - undoManager.addEdit(ce); - } + if (ce.hasEdits()) { + undoManager.addEdit(ce); + } - if (!failures.isEmpty()) { - showFailures(failures); - } + if (!failures.isEmpty()) { + showFailures(failures); } + } - private void showFailures(List failures) { - String message = failures.stream() - .map(exception -> "- " + exception.getLocalizedMessage()) - .collect(Collectors.joining("\n")); + private void showFailures(List failures) { + String message = failures.stream() + .map(exception -> "- " + exception.getLocalizedMessage()) + .collect(Collectors.joining("\n")); - Platform.runLater(() -> - dialogService.showErrorDialogAndWait(Localization.lang("File Move Errors"), message) - ); - } + Platform.runLater(() -> + dialogService.showErrorDialogAndWait(Localization.lang("File Move Errors"), message) + ); } +} diff --git a/jablib/src/main/java/org/jabref/logic/cleanup/CleanupPreferences.java b/jablib/src/main/java/org/jabref/logic/cleanup/CleanupPreferences.java index a1117ec5ec9..02b90693fcf 100644 --- a/jablib/src/main/java/org/jabref/logic/cleanup/CleanupPreferences.java +++ b/jablib/src/main/java/org/jabref/logic/cleanup/CleanupPreferences.java @@ -73,6 +73,11 @@ public void setFieldFormatterCleanups(FieldFormatterCleanups fieldFormatters) { fieldFormatterCleanups.setValue(fieldFormatters); } + /* + * Categories are used to group CleanupSteps by tab type. This allows the updateWith() + * method to replace only the steps of the same category when merging preferences from + * a single tab, without affecting other categories. + */ public CleanupPreferences updateWith(CleanupPreferences tabPreferences) { EnumSet mergedJobs = getActiveJobs(); From 033201c6e4271970f2b021695b83656b6496d608 Mon Sep 17 00:00:00 2001 From: Alex Sukhin Date: Thu, 11 Sep 2025 01:35:29 +0100 Subject: [PATCH 03/27] Fix Trag-bot review issues - Removed trivial comments - Renamed PDF-related variables - Updated methods to return Optional - Used Optional property for FieldFormatterCleanups --- .../org/jabref/gui/cleanup/CleanupDialog.java | 3 +- .../gui/cleanup/CleanupFileRelatedPanel.java | 40 ++++++++++--------- .../gui/cleanup/CleanupMultiFieldPanel.java | 9 +++-- .../org/jabref/gui/cleanup/CleanupPanel.java | 4 +- .../gui/cleanup/CleanupSingleFieldPanel.java | 9 ++++- .../gui/cleanup/CleanupFileRelatedPanel.fxml | 8 ++-- .../logic/cleanup/CleanupPreferences.java | 10 +++-- .../main/resources/l10n/JabRef_en.properties | 2 + 8 files changed, 50 insertions(+), 35 deletions(-) diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java index 068c7a82cef..d7713cf083a 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java @@ -20,7 +20,6 @@ public class CleanupDialog extends BaseDialog { public CleanupDialog(BibDatabaseContext databaseContext, CleanupPreferences initialPreset, FilePreferences filePreferences) { setTitle(Localization.lang("Clean up entries")); - // Load FXML ViewLoader.view(this) .load() .setAsDialogPane(this); @@ -39,7 +38,7 @@ public CleanupDialog(BibDatabaseContext databaseContext, CleanupPreferences init if (button.getButtonData() == ButtonBar.ButtonData.OK_DONE) { Tab selectedTab = tabPane.getSelectionModel().getSelectedItem(); CleanupPanel panel = (CleanupPanel) selectedTab.getContent(); - return panel.getCleanupPreferences(); + return panel.getCleanupPreferences().orElse(null); } else { return null; } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java index e7b7674a869..60e446809e5 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java @@ -2,6 +2,7 @@ import java.nio.file.Path; import java.util.EnumSet; +import java.util.Objects; import java.util.Optional; import javafx.fxml.FXML; @@ -19,17 +20,18 @@ public class CleanupFileRelatedPanel extends VBox implements CleanupPanel { - @FXML private Label cleanupRenamePDFLabel; + @FXML private Label cleanupRenamePdfLabel; - @FXML private CheckBox cleanUpMovePDF; + @FXML private CheckBox cleanUpMovePdf; @FXML private CheckBox cleanUpMakePathsRelative; - @FXML private CheckBox cleanUpRenamePDF; - @FXML private CheckBox cleanUpRenamePDFonlyRelativePaths; + @FXML private CheckBox cleanUpRenamePdf; + @FXML private CheckBox cleanUpRenamePdfonlyRelativePaths; @FXML private CheckBox cleanUpDeletedFiles; @FXML private CheckBox cleanUpUpgradeExternalLinks; public CleanupFileRelatedPanel(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences, FilePreferences filePreferences) { - // Load FXML + Objects.requireNonNull(cleanupPreferences, "cleanupPreferences must not be null"); + ViewLoader.view(this) .root(this) .load(); @@ -40,45 +42,45 @@ public CleanupFileRelatedPanel(BibDatabaseContext databaseContext, CleanupPrefer private void init(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences, FilePreferences filePreferences) { Optional firstExistingDir = databaseContext.getFirstExistingFileDir(filePreferences); if (firstExistingDir.isPresent()) { - cleanUpMovePDF.setText(Localization.lang("Move linked files to default file directory %0", firstExistingDir.get().toString())); + cleanUpMovePdf.setText(Localization.lang("Move linked files to default file directory %0", firstExistingDir.get().toString())); } else { - cleanUpMovePDF.setText(Localization.lang("Move linked files to default file directory %0", "...")); + cleanUpMovePdf.setText(Localization.lang("Move linked files to default file directory %0", "...")); // Since the directory does not exist, we cannot move it to there. So, this option is not checked - regardless of the presets stored in the preferences. - cleanUpMovePDF.setDisable(true); - cleanUpMovePDF.setSelected(false); + cleanUpMovePdf.setDisable(true); + cleanUpMovePdf.setSelected(false); } - cleanUpRenamePDFonlyRelativePaths.disableProperty().bind(cleanUpRenamePDF.selectedProperty().not()); + cleanUpRenamePdfonlyRelativePaths.disableProperty().bind(cleanUpRenamePdf.selectedProperty().not()); cleanUpUpgradeExternalLinks.setText(Localization.lang("Upgrade external PDF/PS links to use the '%0' field.", StandardField.FILE.getDisplayName())); String currentPattern = Localization.lang("Filename format pattern (from preferences)") .concat(filePreferences.getFileNamePattern()); - cleanupRenamePDFLabel.setText(currentPattern); + cleanupRenamePdfLabel.setText(currentPattern); updateDisplay(cleanupPreferences); } private void updateDisplay(CleanupPreferences preset) { - if (!cleanUpMovePDF.isDisabled()) { - cleanUpMovePDF.setSelected(preset.isActive(CleanupPreferences.CleanupStep.MOVE_PDF)); + if (!cleanUpMovePdf.isDisabled()) { + cleanUpMovePdf.setSelected(preset.isActive(CleanupPreferences.CleanupStep.MOVE_PDF)); } cleanUpMakePathsRelative.setSelected(preset.isActive(CleanupPreferences.CleanupStep.MAKE_PATHS_RELATIVE)); - cleanUpRenamePDF.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF)); - cleanUpRenamePDFonlyRelativePaths.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS)); + cleanUpRenamePdf.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF)); + cleanUpRenamePdfonlyRelativePaths.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS)); cleanUpUpgradeExternalLinks.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_UPGRADE_EXTERNAL_LINKS)); cleanUpDeletedFiles.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES)); } - public CleanupPreferences getCleanupPreferences() { + public Optional getCleanupPreferences() { EnumSet activeJobs = EnumSet.noneOf(CleanupPreferences.CleanupStep.class); if (cleanUpMakePathsRelative.isSelected()) { activeJobs.add(CleanupPreferences.CleanupStep.MAKE_PATHS_RELATIVE); } - if (cleanUpRenamePDF.isSelected()) { - if (cleanUpRenamePDFonlyRelativePaths.isSelected()) { + if (cleanUpRenamePdf.isSelected()) { + if (cleanUpRenamePdfonlyRelativePaths.isSelected()) { activeJobs.add(CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS); } else { activeJobs.add(CleanupPreferences.CleanupStep.RENAME_PDF); @@ -91,6 +93,6 @@ public CleanupPreferences getCleanupPreferences() { activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES); } - return new CleanupPreferences(activeJobs); + return Optional.of(new CleanupPreferences(activeJobs)); } } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java index f4b23c61ffd..38e7f950fac 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java @@ -1,6 +1,8 @@ package org.jabref.gui.cleanup; import java.util.EnumSet; +import java.util.Objects; +import java.util.Optional; import javafx.fxml.FXML; import javafx.scene.control.CheckBox; @@ -20,7 +22,8 @@ public class CleanupMultiFieldPanel extends VBox implements CleanupPanel { @FXML private CheckBox cleanUpTimestampToModificationDate; public CleanupMultiFieldPanel(CleanupPreferences cleanupPreferences) { - // Load FXML + Objects.requireNonNull(cleanupPreferences, "cleanupPreferences must not be null"); + ViewLoader.view(this) .root(this) .load(); @@ -69,7 +72,7 @@ private void updateDisplay(CleanupPreferences preset) { } @Override - public CleanupPreferences getCleanupPreferences() { + public Optional getCleanupPreferences() { EnumSet activeJobs = EnumSet.noneOf(CleanupPreferences.CleanupStep.class); if (cleanUpDOI.isSelected()) { @@ -94,6 +97,6 @@ public CleanupPreferences getCleanupPreferences() { activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_MODIFICATIONDATE); } - return new CleanupPreferences(activeJobs); + return Optional.of(new CleanupPreferences(activeJobs)); } } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java index 0d64e172a2e..b0ce9718cab 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java @@ -1,7 +1,9 @@ package org.jabref.gui.cleanup; +import java.util.Optional; + import org.jabref.logic.cleanup.CleanupPreferences; public interface CleanupPanel { - CleanupPreferences getCleanupPreferences(); + Optional getCleanupPreferences(); } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java index da3e3d7b9ab..d9478b66391 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java @@ -1,5 +1,8 @@ package org.jabref.gui.cleanup; +import java.util.Objects; +import java.util.Optional; + import javafx.collections.FXCollections; import javafx.fxml.FXML; import javafx.scene.layout.VBox; @@ -15,6 +18,8 @@ public class CleanupSingleFieldPanel extends VBox implements CleanupPanel { @FXML private FieldFormatterCleanupsPanel formatterCleanupsPanel; public CleanupSingleFieldPanel(CleanupPreferences cleanupPreferences) { + Objects.requireNonNull(cleanupPreferences, "cleanupPreferences must not be null"); + ViewLoader.view(this) .root(this) .load(); @@ -30,11 +35,11 @@ private void init(CleanupPreferences cleanupPreferences) { } @Override - public CleanupPreferences getCleanupPreferences() { + public Optional getCleanupPreferences() { FieldFormatterCleanups fieldFormatterCleanups = new FieldFormatterCleanups( !formatterCleanupsPanel.cleanupsDisableProperty().getValue(), formatterCleanupsPanel.cleanupsProperty() ); - return new CleanupPreferences(fieldFormatterCleanups); + return Optional.of(new CleanupPreferences(fieldFormatterCleanups)); } } diff --git a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml index 49af70a6003..368d31e3bed 100644 --- a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml +++ b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml @@ -17,12 +17,12 @@ - + - + - diff --git a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupMultiFieldPanel.fxml b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupMultiFieldPanel.fxml index a6bf6f1adad..f05aaf370f1 100644 --- a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupMultiFieldPanel.fxml +++ b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupMultiFieldPanel.fxml @@ -15,12 +15,12 @@ - - - - - - - + + + + + + + From 4f2ed12e5256a7c8f98dfdb35f4c965b12837b02 Mon Sep 17 00:00:00 2001 From: Alex Sukhin Date: Thu, 11 Sep 2025 17:57:39 +0100 Subject: [PATCH 10/27] Returns CleanupPreferences directly since value is always present --- .../src/main/java/org/jabref/gui/cleanup/CleanupDialog.java | 2 +- .../java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java | 4 ++-- .../java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java | 5 ++--- .../src/main/java/org/jabref/gui/cleanup/CleanupPanel.java | 4 +--- .../java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java | 5 ++--- 5 files changed, 8 insertions(+), 12 deletions(-) diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java index d7713cf083a..c948dc03df7 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java @@ -38,7 +38,7 @@ public CleanupDialog(BibDatabaseContext databaseContext, CleanupPreferences init if (button.getButtonData() == ButtonBar.ButtonData.OK_DONE) { Tab selectedTab = tabPane.getSelectionModel().getSelectedItem(); CleanupPanel panel = (CleanupPanel) selectedTab.getContent(); - return panel.getCleanupPreferences().orElse(null); + return panel.getCleanupPreferences(); } else { return null; } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java index 3ee263e824a..8b171401fa2 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java @@ -74,7 +74,7 @@ private void updateDisplay(CleanupPreferences preset) { cleanupDeletedFiles.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES)); } - public Optional getCleanupPreferences() { + public CleanupPreferences getCleanupPreferences() { EnumSet activeJobs = EnumSet.noneOf(CleanupPreferences.CleanupStep.class); if (cleanupMakePathsRelative.isSelected()) { @@ -94,6 +94,6 @@ public Optional getCleanupPreferences() { activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES); } - return Optional.of(new CleanupPreferences(activeJobs)); + return new CleanupPreferences(activeJobs); } } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java index 0f7bf4f7a15..a4d7dd03877 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java @@ -2,7 +2,6 @@ import java.util.EnumSet; import java.util.Objects; -import java.util.Optional; import javafx.fxml.FXML; import javafx.scene.control.CheckBox; @@ -72,7 +71,7 @@ private void updateDisplay(CleanupPreferences preset) { } @Override - public Optional getCleanupPreferences() { + public CleanupPreferences getCleanupPreferences() { EnumSet activeJobs = EnumSet.noneOf(CleanupPreferences.CleanupStep.class); if (cleanupDOI.isSelected()) { @@ -97,6 +96,6 @@ public Optional getCleanupPreferences() { activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_MODIFICATIONDATE); } - return Optional.of(new CleanupPreferences(activeJobs)); + return new CleanupPreferences(activeJobs); } } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java index b0ce9718cab..0d64e172a2e 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java @@ -1,9 +1,7 @@ package org.jabref.gui.cleanup; -import java.util.Optional; - import org.jabref.logic.cleanup.CleanupPreferences; public interface CleanupPanel { - Optional getCleanupPreferences(); + CleanupPreferences getCleanupPreferences(); } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java index d9478b66391..f8ca7b7a4ab 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java @@ -1,7 +1,6 @@ package org.jabref.gui.cleanup; import java.util.Objects; -import java.util.Optional; import javafx.collections.FXCollections; import javafx.fxml.FXML; @@ -35,11 +34,11 @@ private void init(CleanupPreferences cleanupPreferences) { } @Override - public Optional getCleanupPreferences() { + public CleanupPreferences getCleanupPreferences() { FieldFormatterCleanups fieldFormatterCleanups = new FieldFormatterCleanups( !formatterCleanupsPanel.cleanupsDisableProperty().getValue(), formatterCleanupsPanel.cleanupsProperty() ); - return Optional.of(new CleanupPreferences(fieldFormatterCleanups)); + return new CleanupPreferences(fieldFormatterCleanups); } } From 1d2f99175eab47a867032ac2adcc361ff9fe2f55 Mon Sep 17 00:00:00 2001 From: Alex Sukhin Date: Sun, 14 Sep 2025 03:24:12 +0100 Subject: [PATCH 11/27] Initial review refactor draft - Create new ViewModel, pull logic from Action and SingleAction into ViewModel - Move Apply button to each tab - Remove categories from ENUM and keep enums of all jobs in each respective tab to be used for cleanup --- .../org/jabref/gui/cleanup/CleanupAction.java | 140 +----------- .../org/jabref/gui/cleanup/CleanupDialog.java | 83 +++++-- .../gui/cleanup/CleanupDialogViewModel.java | 216 ++++++++++++++++++ .../gui/cleanup/CleanupFileRelatedPanel.java | 33 ++- .../gui/cleanup/CleanupMultiFieldPanel.java | 32 ++- .../org/jabref/gui/cleanup/CleanupPanel.java | 7 - .../gui/cleanup/CleanupSingleAction.java | 94 +------- .../gui/cleanup/CleanupSingleFieldPanel.java | 23 +- .../gui/cleanup/CleanupTabSelection.java | 28 +++ .../org/jabref/gui/cleanup/CleanupDialog.fxml | 1 - .../gui/cleanup/CleanupFileRelatedPanel.fxml | 4 + .../gui/cleanup/CleanupMultiFieldPanel.fxml | 4 + .../gui/cleanup/CleanupSingleFieldPanel.fxml | 4 + .../logic/cleanup/CleanupPreferences.java | 98 +++----- .../logic/cleanup/CleanupPreferencesTest.java | 35 ++- 15 files changed, 479 insertions(+), 323 deletions(-) create mode 100644 jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialogViewModel.java delete mode 100644 jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java create mode 100644 jabgui/src/main/java/org/jabref/gui/cleanup/CleanupTabSelection.java diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java index 0149a040dc6..a2f511bdb0a 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupAction.java @@ -1,32 +1,16 @@ package org.jabref.gui.cleanup; -import java.util.ArrayList; -import java.util.List; -import java.util.Optional; import java.util.function.Supplier; -import java.util.stream.Collectors; import javax.swing.undo.UndoManager; -import javafx.application.Platform; - import org.jabref.gui.DialogService; import org.jabref.gui.LibraryTab; import org.jabref.gui.StateManager; import org.jabref.gui.actions.ActionHelper; import org.jabref.gui.actions.SimpleCommand; -import org.jabref.gui.undo.NamedCompound; -import org.jabref.gui.undo.UndoableFieldChange; -import org.jabref.logic.JabRefException; -import org.jabref.logic.cleanup.CleanupPreferences; -import org.jabref.logic.cleanup.CleanupWorker; -import org.jabref.logic.l10n.Localization; import org.jabref.logic.preferences.CliPreferences; -import org.jabref.logic.util.BackgroundTask; import org.jabref.logic.util.TaskExecutor; -import org.jabref.model.FieldChange; -import org.jabref.model.database.BibDatabaseContext; -import org.jabref.model.entry.BibEntry; public class CleanupAction extends SimpleCommand { @@ -36,10 +20,6 @@ public class CleanupAction extends SimpleCommand { private final StateManager stateManager; private final TaskExecutor taskExecutor; private final UndoManager undoManager; - private final List failures; - - private boolean isCanceled; - private int modifiedEntriesCount; public CleanupAction(Supplier tabSupplier, CliPreferences preferences, @@ -53,7 +33,6 @@ public CleanupAction(Supplier tabSupplier, this.stateManager = stateManager; this.taskExecutor = taskExecutor; this.undoManager = undoManager; - this.failures = new ArrayList<>(); this.executable.bind(ActionHelper.needsEntriesSelected(stateManager)); } @@ -64,121 +43,16 @@ public void execute() { return; } - if (stateManager.getSelectedEntries().isEmpty()) { // None selected. Inform the user to select entries first. - dialogService.showInformationDialogAndWait(Localization.lang("Cleanup entry"), Localization.lang("First select entries to clean up.")); - return; - } - - isCanceled = false; - modifiedEntriesCount = 0; - CleanupDialog cleanupDialog = new CleanupDialog( + tabSupplier, stateManager.getActiveDatabase().get(), - preferences.getCleanupPreferences(), - preferences.getFilePreferences() - ); - - Optional chosenPreset = dialogService.showCustomDialogAndWait(cleanupDialog); - - chosenPreset.ifPresent(preset -> { - if (preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF) && preferences.getAutoLinkPreferences().shouldAskAutoNamingPdfs()) { - boolean confirmed = dialogService.showConfirmationDialogWithOptOutAndWait(Localization.lang("Autogenerate PDF Names"), - Localization.lang("Auto-generating PDF-Names does not support undo. Continue?"), - Localization.lang("Autogenerate PDF Names"), - Localization.lang("Cancel"), - Localization.lang("Do not ask again"), - optOut -> preferences.getAutoLinkPreferences().setAskAutoNamingPdfs(!optOut)); - if (!confirmed) { - isCanceled = true; - return; - } - } - - CleanupPreferences updatedPreferences = preferences.getCleanupPreferences().updateWith(preset); - - preferences.getCleanupPreferences().setActiveJobs(updatedPreferences.getActiveJobs()); - preferences.getCleanupPreferences().setFieldFormatterCleanups(updatedPreferences.getFieldFormatterCleanups()); - - BackgroundTask.wrap(() -> cleanup(stateManager.getActiveDatabase().get(), preset)) - .onSuccess(result -> showResults()) - .onFailure(dialogService::showErrorDialogAndWait) - .executeWith(taskExecutor); - }); - } - - /** - * Runs the cleanup on the entry and records the change. - * - * @return true iff entry was modified - */ - private boolean doCleanup(BibDatabaseContext databaseContext, CleanupPreferences preset, BibEntry entry, NamedCompound ce) { - // Create and run cleaner - CleanupWorker cleaner = new CleanupWorker( - databaseContext, - preferences.getFilePreferences(), - preferences.getTimestampPreferences() + preferences, + dialogService, + stateManager, + taskExecutor, + undoManager ); - List changes = cleaner.cleanup(preset, entry); - - // Register undo action - for (FieldChange change : changes) { - ce.addEdit(new UndoableFieldChange(change)); - } - - failures.addAll(cleaner.getFailures()); - - return !changes.isEmpty(); - } - - private void showResults() { - if (isCanceled) { - return; - } - - if (modifiedEntriesCount > 0) { - tabSupplier.get().markBaseChanged(); - } - - if (modifiedEntriesCount == 0) { - dialogService.notify(Localization.lang("No entry needed a clean up")); - } else if (modifiedEntriesCount == 1) { - dialogService.notify(Localization.lang("One entry needed a clean up")); - } else { - dialogService.notify(Localization.lang("%0 entries needed a clean up", Integer.toString(modifiedEntriesCount))); - } - } - - private void cleanup(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences) { - this.failures.clear(); - - // undo granularity is on set of all entries - NamedCompound ce = new NamedCompound(Localization.lang("Clean up entries")); - - for (BibEntry entry : List.copyOf(stateManager.getSelectedEntries())) { - if (doCleanup(databaseContext, cleanupPreferences, entry, ce)) { - modifiedEntriesCount++; - } - } - - ce.end(); - - if (ce.hasEdits()) { - undoManager.addEdit(ce); - } - - if (!failures.isEmpty()) { - showFailures(failures); - } - } - - private void showFailures(List failures) { - String message = failures.stream() - .map(exception -> "- " + exception.getLocalizedMessage()) - .collect(Collectors.joining("\n")); - - Platform.runLater(() -> - dialogService.showErrorDialogAndWait(Localization.lang("File Move Errors"), message) - ); + dialogService.showCustomDialogAndWait(cleanupDialog); } } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java index c948dc03df7..d154cc6a8d8 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialog.java @@ -1,47 +1,96 @@ package org.jabref.gui.cleanup; +import java.util.List; +import java.util.Optional; +import java.util.function.Supplier; + +import javax.swing.undo.UndoManager; + import javafx.fxml.FXML; -import javafx.scene.control.ButtonBar; import javafx.scene.control.Tab; import javafx.scene.control.TabPane; +import org.jabref.gui.DialogService; +import org.jabref.gui.LibraryTab; +import org.jabref.gui.StateManager; import org.jabref.gui.util.BaseDialog; import org.jabref.logic.FilePreferences; import org.jabref.logic.cleanup.CleanupPreferences; import org.jabref.logic.l10n.Localization; +import org.jabref.logic.preferences.CliPreferences; +import org.jabref.logic.util.TaskExecutor; import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.BibEntry; import com.airhacks.afterburner.views.ViewLoader; -public class CleanupDialog extends BaseDialog { +public class CleanupDialog extends BaseDialog { @FXML private TabPane tabPane; - public CleanupDialog(BibDatabaseContext databaseContext, CleanupPreferences initialPreset, FilePreferences filePreferences) { + private final CleanupDialogViewModel viewModel; + + // Constructor for multiple-entry cleanup + public CleanupDialog(Supplier tabSupplier, + BibDatabaseContext databaseContext, + CliPreferences preferences, + DialogService dialogService, + StateManager stateManager, + TaskExecutor taskExecutor, + UndoManager undoManager) { + + this.viewModel = new CleanupDialogViewModel( + Optional.of(tabSupplier), + databaseContext, + preferences, + dialogService, + stateManager, + Optional.of(taskExecutor), + undoManager + ); + + init(databaseContext, preferences); + } + + // Constructor for single-entry cleanup + public CleanupDialog(BibEntry targetEntry, + BibDatabaseContext databaseContext, + CliPreferences preferences, + DialogService dialogService, + StateManager stateManager, + UndoManager undoManager) { + + this.viewModel = new CleanupDialogViewModel( + databaseContext, + preferences, + dialogService, + stateManager, + undoManager + ); + + init(databaseContext, preferences); + + viewModel.setTargetEntries(Optional.of(List.of(targetEntry))); + } + + private void init(BibDatabaseContext databaseContext, CliPreferences preferences) { setTitle(Localization.lang("Clean up entries")); ViewLoader.view(this) .load() .setAsDialogPane(this); - CleanupSingleFieldPanel singleFieldPanel = new CleanupSingleFieldPanel(initialPreset); - CleanupFileRelatedPanel fileRelatedPanel = new CleanupFileRelatedPanel(databaseContext, initialPreset, filePreferences); - CleanupMultiFieldPanel multiFieldPanel = new CleanupMultiFieldPanel(initialPreset); + CleanupPreferences initialPreset = preferences.getCleanupPreferences(); + FilePreferences filePreferences = preferences.getFilePreferences(); - tabPane.getTabs().addAll( + CleanupSingleFieldPanel singleFieldPanel = new CleanupSingleFieldPanel(initialPreset, viewModel); + CleanupFileRelatedPanel fileRelatedPanel = new CleanupFileRelatedPanel(databaseContext, initialPreset, filePreferences, viewModel); + CleanupMultiFieldPanel multiFieldPanel = new CleanupMultiFieldPanel(initialPreset, viewModel); + + tabPane.getTabs().setAll( new Tab(Localization.lang("Single field"), singleFieldPanel), new Tab(Localization.lang("File-related"), fileRelatedPanel), new Tab(Localization.lang("Multi-field"), multiFieldPanel) ); - - setResultConverter(button -> { - if (button.getButtonData() == ButtonBar.ButtonData.OK_DONE) { - Tab selectedTab = tabPane.getSelectionModel().getSelectedItem(); - CleanupPanel panel = (CleanupPanel) selectedTab.getContent(); - return panel.getCleanupPreferences(); - } else { - return null; - } - }); } } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialogViewModel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialogViewModel.java new file mode 100644 index 00000000000..f302233cd7e --- /dev/null +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupDialogViewModel.java @@ -0,0 +1,216 @@ +package org.jabref.gui.cleanup; + +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +import javax.swing.undo.UndoManager; + +import javafx.application.Platform; + +import org.jabref.gui.AbstractViewModel; +import org.jabref.gui.DialogService; +import org.jabref.gui.LibraryTab; +import org.jabref.gui.StateManager; +import org.jabref.gui.undo.NamedCompound; +import org.jabref.gui.undo.UndoableFieldChange; +import org.jabref.logic.JabRefException; +import org.jabref.logic.cleanup.CleanupPreferences; +import org.jabref.logic.cleanup.CleanupWorker; +import org.jabref.logic.l10n.Localization; +import org.jabref.logic.preferences.CliPreferences; +import org.jabref.logic.util.BackgroundTask; +import org.jabref.logic.util.TaskExecutor; +import org.jabref.model.FieldChange; +import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.BibEntry; + +public class CleanupDialogViewModel extends AbstractViewModel { + + private final Optional> tabSupplier; + private final BibDatabaseContext databaseContext; + private final CliPreferences preferences; + private final DialogService dialogService; + private final StateManager stateManager; + private final Optional taskExecutor; + private final UndoManager undoManager; + + private final List failures; + private Optional> targetEntries; + + private boolean isCanceled; + private int modifiedEntriesCount; + + public CleanupDialogViewModel(Optional> tabSupplier, + BibDatabaseContext databaseContext, + CliPreferences preferences, + DialogService dialogService, + StateManager stateManager, + Optional taskExecutor, + UndoManager undoManager) { + this.databaseContext = Objects.requireNonNull(databaseContext, "databaseContext must not be null"); + this.preferences = Objects.requireNonNull(preferences, "preferences must not be null"); + this.dialogService = Objects.requireNonNull(dialogService, "dialogService must not be null"); + this.stateManager = Objects.requireNonNull(stateManager, "stateManager must not be null"); + this.undoManager = Objects.requireNonNull(undoManager, "undoManager must not be null"); + + this.tabSupplier = (tabSupplier.isEmpty()) ? Optional.empty() : tabSupplier; + this.taskExecutor = (taskExecutor.isEmpty()) ? Optional.empty() : taskExecutor; + + this.failures = new ArrayList<>(); + this.targetEntries = Optional.empty(); + } + + public CleanupDialogViewModel(BibDatabaseContext databaseContext, + CliPreferences preferences, + DialogService dialogService, + StateManager stateManager, + UndoManager undoManager) { + this( + Optional.empty(), + databaseContext, + preferences, + dialogService, + stateManager, + Optional.empty(), + undoManager + ); + } + + public void setTargetEntries(Optional> entries) { + this.targetEntries = (entries.isEmpty()) ? Optional.empty() : entries.map(List::copyOf); + } + + public void apply(CleanupTabSelection selectedTab) { + if (stateManager.getActiveDatabase().isEmpty()) { + return; + } + + List entriesToProcess = targetEntries + .orElseGet(() -> List.copyOf(stateManager.getSelectedEntries())); + + if (entriesToProcess.isEmpty()) { // None selected. Inform the user to select entries first. + dialogService.showInformationDialogAndWait(Localization.lang("Cleanup entry"), Localization.lang("First select entries to clean up.") + ); + return; + } + + isCanceled = false; + modifiedEntriesCount = 0; + + if (selectedTab.selectedJobs().contains(CleanupPreferences.CleanupStep.RENAME_PDF) && preferences.getAutoLinkPreferences().shouldAskAutoNamingPdfs()) { + boolean confirmed = dialogService.showConfirmationDialogWithOptOutAndWait( + Localization.lang("Autogenerate PDF Names"), + Localization.lang("Auto-generating PDF-Names does not support undo. Continue?"), + Localization.lang("Autogenerate PDF Names"), + Localization.lang("Cancel"), + Localization.lang("Do not ask again"), + optOut -> preferences.getAutoLinkPreferences().setAskAutoNamingPdfs(!optOut) + ); + if (!confirmed) { + isCanceled = true; + return; + } + } + + CleanupPreferences updatedPreferences = preferences.getCleanupPreferences() + .updateWith(Optional.ofNullable(selectedTab.allJobs()), Optional.of(selectedTab.selectedJobs()), selectedTab.formatters()); + + preferences.getCleanupPreferences().setActiveJobs(updatedPreferences.getActiveJobs()); + preferences.getCleanupPreferences().setFieldFormatterCleanups(updatedPreferences.getFieldFormatterCleanups()); + + CleanupPreferences runPreset = new CleanupPreferences(EnumSet.copyOf(selectedTab.selectedJobs())); + selectedTab.formatters().ifPresent(runPreset::setFieldFormatterCleanups); + + taskExecutor.ifPresentOrElse( + executor -> BackgroundTask.wrap(() -> cleanup(databaseContext, runPreset, entriesToProcess)) + .onSuccess(result -> showResults()) + .onFailure(dialogService::showErrorDialogAndWait) + .executeWith(executor), + () -> { + cleanup(databaseContext, runPreset, entriesToProcess); + } + ); + } + + /** + * Runs the cleanup on the entry and records the change. + * + * @return true iff entry was modified + */ + private boolean doCleanup(BibDatabaseContext databaseContext, CleanupPreferences preset, BibEntry entry, NamedCompound compound) { + // Create and run cleaner + CleanupWorker cleaner = new CleanupWorker( + databaseContext, + preferences.getFilePreferences(), + preferences.getTimestampPreferences() + ); + + List changes = cleaner.cleanup(preset, entry); + + // Register undo action + for (FieldChange change : changes) { + compound.addEdit(new UndoableFieldChange(change)); + } + + failures.addAll(cleaner.getFailures()); + + return !changes.isEmpty(); + } + + private void showResults() { + if (isCanceled) { + return; + } + + if (modifiedEntriesCount > 0) { + tabSupplier.ifPresent(s -> s.get().markBaseChanged()); + } + + if (modifiedEntriesCount == 0) { + dialogService.notify(Localization.lang("No entry needed a clean up")); + } else if (modifiedEntriesCount == 1) { + dialogService.notify(Localization.lang("One entry needed a clean up")); + } else { + dialogService.notify(Localization.lang("%0 entries needed a clean up", Integer.toString(modifiedEntriesCount))); + } + } + + private void cleanup(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences, List entries) { + this.failures.clear(); + + // undo granularity is on a set of all entries + NamedCompound compound = new NamedCompound(Localization.lang("Clean up entries")); + + for (BibEntry entry : entries) { + if (doCleanup(databaseContext, cleanupPreferences, entry, compound)) { + modifiedEntriesCount++; + } + } + + compound.end(); + + if (compound.hasEdits()) { + undoManager.addEdit(compound); + } + + if (!failures.isEmpty()) { + showFailures(failures); + } + } + + private void showFailures(List failures) { + String message = failures.stream() + .map(exception -> "- " + exception.getLocalizedMessage()) + .collect(Collectors.joining("\n")); + + Platform.runLater(() -> + dialogService.showErrorDialogAndWait(Localization.lang("File Move Errors"), message) + ); + } +} + diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java index 8b171401fa2..e3fc780ef3d 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupFileRelatedPanel.java @@ -18,7 +18,7 @@ import com.airhacks.afterburner.views.ViewLoader; -public class CleanupFileRelatedPanel extends VBox implements CleanupPanel { +public class CleanupFileRelatedPanel extends VBox { @FXML private Label cleanupRenamePdfLabel; @@ -29,10 +29,28 @@ public class CleanupFileRelatedPanel extends VBox implements CleanupPanel { @FXML private CheckBox cleanupDeletedFiles; @FXML private CheckBox cleanupUpgradeExternalLinks; - public CleanupFileRelatedPanel(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences, FilePreferences filePreferences) { + private final EnumSet ALL_JOBS = EnumSet.of( + CleanupPreferences.CleanupStep.MOVE_PDF, + CleanupPreferences.CleanupStep.MAKE_PATHS_RELATIVE, + CleanupPreferences.CleanupStep.RENAME_PDF, + CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS, + CleanupPreferences.CleanupStep.CLEAN_UP_UPGRADE_EXTERNAL_LINKS, + CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES + ); + + + private final CleanupDialogViewModel viewModel; + + public CleanupFileRelatedPanel(BibDatabaseContext databaseContext, + CleanupPreferences cleanupPreferences, + FilePreferences filePreferences, + CleanupDialogViewModel viewModel) { Objects.requireNonNull(databaseContext, "databaseContext must not be null"); Objects.requireNonNull(cleanupPreferences, "cleanupPreferences must not be null"); Objects.requireNonNull(filePreferences, "filePreferences must not be null"); + Objects.requireNonNull(viewModel, "viewModel must not be null"); + + this.viewModel = viewModel; ViewLoader.view(this) .root(this) @@ -41,6 +59,13 @@ public CleanupFileRelatedPanel(BibDatabaseContext databaseContext, CleanupPrefer init(databaseContext, cleanupPreferences, filePreferences); } + @FXML + private void onApply() { + CleanupTabSelection selectedTab = CleanupTabSelection.ofJobs(ALL_JOBS, getSelectedJobs()); + viewModel.apply(selectedTab); + getScene().getWindow().hide(); + } + private void init(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences, FilePreferences filePreferences) { Optional firstExistingDir = databaseContext.getFirstExistingFileDir(filePreferences); if (firstExistingDir.isPresent()) { @@ -74,7 +99,7 @@ private void updateDisplay(CleanupPreferences preset) { cleanupDeletedFiles.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES)); } - public CleanupPreferences getCleanupPreferences() { + public EnumSet getSelectedJobs() { EnumSet activeJobs = EnumSet.noneOf(CleanupPreferences.CleanupStep.class); if (cleanupMakePathsRelative.isSelected()) { @@ -94,6 +119,6 @@ public CleanupPreferences getCleanupPreferences() { activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES); } - return new CleanupPreferences(activeJobs); + return activeJobs; } } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java index a4d7dd03877..0844971bff9 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupMultiFieldPanel.java @@ -11,7 +11,7 @@ import com.airhacks.afterburner.views.ViewLoader; -public class CleanupMultiFieldPanel extends VBox implements CleanupPanel { +public class CleanupMultiFieldPanel extends VBox { @FXML private CheckBox cleanupDOI; @FXML private CheckBox cleanupEprint; @FXML private CheckBox cleanupURL; @@ -20,8 +20,24 @@ public class CleanupMultiFieldPanel extends VBox implements CleanupPanel { @FXML private CheckBox cleanupTimestampToCreationDate; @FXML private CheckBox cleanupTimestampToModificationDate; - public CleanupMultiFieldPanel(CleanupPreferences cleanupPreferences) { + private final EnumSet ALL_JOBS = EnumSet.of( + CleanupPreferences.CleanupStep.CLEAN_UP_DOI, + CleanupPreferences.CleanupStep.CLEANUP_EPRINT, + CleanupPreferences.CleanupStep.CLEAN_UP_URL, + CleanupPreferences.CleanupStep.CONVERT_TO_BIBLATEX, + CleanupPreferences.CleanupStep.CONVERT_TO_BIBTEX, + CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_CREATIONDATE, + CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_MODIFICATIONDATE + ); + + private final CleanupDialogViewModel viewModel; + + public CleanupMultiFieldPanel(CleanupPreferences cleanupPreferences, + CleanupDialogViewModel viewModel) { Objects.requireNonNull(cleanupPreferences, "cleanupPreferences must not be null"); + Objects.requireNonNull(viewModel, "viewModel must not be null"); + + this.viewModel = viewModel; ViewLoader.view(this) .root(this) @@ -30,6 +46,13 @@ public CleanupMultiFieldPanel(CleanupPreferences cleanupPreferences) { init(cleanupPreferences); } + @FXML + private void onApply() { + CleanupTabSelection selectedTab = CleanupTabSelection.ofJobs(ALL_JOBS, getSelectedJobs()); + viewModel.apply(selectedTab); + getScene().getWindow().hide(); + } + private void init(CleanupPreferences cleanupPreferences) { cleanupBibtex.selectedProperty().addListener( (observable, oldValue, newValue) -> { @@ -70,8 +93,7 @@ private void updateDisplay(CleanupPreferences preset) { cleanupTimestampToModificationDate.setSelected(preset.isActive(CleanupPreferences.CleanupStep.DO_NOT_CONVERT_TIMESTAMP)); } - @Override - public CleanupPreferences getCleanupPreferences() { + public EnumSet getSelectedJobs() { EnumSet activeJobs = EnumSet.noneOf(CleanupPreferences.CleanupStep.class); if (cleanupDOI.isSelected()) { @@ -96,6 +118,6 @@ public CleanupPreferences getCleanupPreferences() { activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_MODIFICATIONDATE); } - return new CleanupPreferences(activeJobs); + return activeJobs; } } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java deleted file mode 100644 index 0d64e172a2e..00000000000 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupPanel.java +++ /dev/null @@ -1,7 +0,0 @@ -package org.jabref.gui.cleanup; - -import org.jabref.logic.cleanup.CleanupPreferences; - -public interface CleanupPanel { - CleanupPreferences getCleanupPreferences(); -} diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleAction.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleAction.java index 6f9ffc08892..37e1442ff15 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleAction.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleAction.java @@ -1,25 +1,12 @@ package org.jabref.gui.cleanup; -import java.util.List; -import java.util.Optional; - import javax.swing.undo.UndoManager; -import javafx.application.Platform; - import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; import org.jabref.gui.actions.ActionHelper; import org.jabref.gui.actions.SimpleCommand; -import org.jabref.gui.undo.NamedCompound; -import org.jabref.gui.undo.UndoableFieldChange; -import org.jabref.logic.JabRefException; -import org.jabref.logic.cleanup.CleanupPreferences; -import org.jabref.logic.cleanup.CleanupWorker; -import org.jabref.logic.l10n.Localization; import org.jabref.logic.preferences.CliPreferences; -import org.jabref.model.FieldChange; -import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; public class CleanupSingleAction extends SimpleCommand { @@ -30,9 +17,6 @@ public class CleanupSingleAction extends SimpleCommand { private final BibEntry entry; private final UndoManager undoManager; - private boolean isCanceled; - private int modifiedEntriesCount; - public CleanupSingleAction(BibEntry entry, CliPreferences preferences, DialogService dialogService, StateManager stateManager, UndoManager undoManager) { this.entry = entry; this.preferences = preferences; @@ -45,79 +29,19 @@ public CleanupSingleAction(BibEntry entry, CliPreferences preferences, DialogSer @Override public void execute() { - isCanceled = false; + if (stateManager.getActiveDatabase().isEmpty()) { + return; + } CleanupDialog cleanupDialog = new CleanupDialog( + entry, stateManager.getActiveDatabase().get(), - preferences.getCleanupPreferences(), - preferences.getFilePreferences() - ); - - Optional chosenPreset = dialogService.showCustomDialogAndWait(cleanupDialog); - - chosenPreset.ifPresent(preset -> { - if (preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF) && preferences.getAutoLinkPreferences().shouldAskAutoNamingPdfs()) { - boolean confirmed = dialogService.showConfirmationDialogWithOptOutAndWait(Localization.lang("Autogenerate PDF Names"), - Localization.lang("Auto-generating PDF-Names does not support undo. Continue?"), - Localization.lang("Autogenerate PDF Names"), - Localization.lang("Cancel"), - Localization.lang("Do not ask again"), - optOut -> preferences.getAutoLinkPreferences().setAskAutoNamingPdfs(!optOut)); - if (!confirmed) { - isCanceled = true; - return; - } - } - - preferences.getCleanupPreferences().setActiveJobs(preset.getActiveJobs()); - preferences.getCleanupPreferences().setFieldFormatterCleanups(preset.getFieldFormatterCleanups()); - - cleanup(stateManager.getActiveDatabase().get(), preset); - }); - } - - /** - * Runs the cleanup on the entry and records the change. - */ - private void doCleanup(BibDatabaseContext databaseContext, CleanupPreferences preset, BibEntry entry, NamedCompound ce) { - // Create and run cleaner - CleanupWorker cleaner = new CleanupWorker( - databaseContext, - preferences.getFilePreferences(), - preferences.getTimestampPreferences() + preferences, + dialogService, + stateManager, + undoManager ); - List changes = cleaner.cleanup(preset, entry); - - // Register undo action - for (FieldChange change : changes) { - ce.addEdit(new UndoableFieldChange(change)); - } - - if (!cleaner.getFailures().isEmpty()) { - this.showFailures(cleaner.getFailures()); - } - } - - private void cleanup(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences) { - // undo granularity is on entry level - NamedCompound ce = new NamedCompound(Localization.lang("Cleanup entry")); - - doCleanup(databaseContext, cleanupPreferences, entry, ce); - - ce.end(); - if (ce.hasEdits()) { - undoManager.addEdit(ce); - } - } - - private void showFailures(List failures) { - StringBuilder sb = new StringBuilder(); - for (JabRefException exception : failures) { - sb.append("- ").append(exception.getLocalizedMessage()).append("\n"); - } - Platform.runLater(() -> - dialogService.showErrorDialogAndWait(Localization.lang("File Move Errors"), sb.toString()) - ); + dialogService.showCustomDialogAndWait(cleanupDialog); } } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java index f8ca7b7a4ab..b4f4a9fd090 100644 --- a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupSingleFieldPanel.java @@ -12,12 +12,18 @@ import com.airhacks.afterburner.views.ViewLoader; -public class CleanupSingleFieldPanel extends VBox implements CleanupPanel { +public class CleanupSingleFieldPanel extends VBox { @FXML private FieldFormatterCleanupsPanel formatterCleanupsPanel; - public CleanupSingleFieldPanel(CleanupPreferences cleanupPreferences) { + private final CleanupDialogViewModel viewModel; + + public CleanupSingleFieldPanel(CleanupPreferences cleanupPreferences, + CleanupDialogViewModel viewModel) { Objects.requireNonNull(cleanupPreferences, "cleanupPreferences must not be null"); + Objects.requireNonNull(viewModel, "viewModel must not be null"); + + this.viewModel = viewModel; ViewLoader.view(this) .root(this) @@ -26,6 +32,13 @@ public CleanupSingleFieldPanel(CleanupPreferences cleanupPreferences) { init(cleanupPreferences); } + @FXML + private void onApply() { + CleanupTabSelection selectedTab = CleanupTabSelection.ofFormatters(getSelectedFormatters()); + viewModel.apply(selectedTab); + getScene().getWindow().hide(); + } + private void init(CleanupPreferences cleanupPreferences) { formatterCleanupsPanel.cleanupsDisableProperty().setValue(!cleanupPreferences.getFieldFormatterCleanups().isEnabled()); formatterCleanupsPanel.cleanupsProperty().setValue(FXCollections.observableArrayList( @@ -33,12 +46,10 @@ private void init(CleanupPreferences cleanupPreferences) { )); } - @Override - public CleanupPreferences getCleanupPreferences() { - FieldFormatterCleanups fieldFormatterCleanups = new FieldFormatterCleanups( + public FieldFormatterCleanups getSelectedFormatters() { + return new FieldFormatterCleanups( !formatterCleanupsPanel.cleanupsDisableProperty().getValue(), formatterCleanupsPanel.cleanupsProperty() ); - return new CleanupPreferences(fieldFormatterCleanups); } } diff --git a/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupTabSelection.java b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupTabSelection.java new file mode 100644 index 00000000000..b617b67d22c --- /dev/null +++ b/jabgui/src/main/java/org/jabref/gui/cleanup/CleanupTabSelection.java @@ -0,0 +1,28 @@ +package org.jabref.gui.cleanup; + +import java.util.EnumSet; +import java.util.Objects; +import java.util.Optional; + +import org.jabref.logic.cleanup.CleanupPreferences; +import org.jabref.logic.cleanup.FieldFormatterCleanups; + +public record CleanupTabSelection( + EnumSet allJobs, + EnumSet selectedJobs, + Optional formatters) { + + public CleanupTabSelection { + allJobs = (allJobs == null) ? EnumSet.noneOf(CleanupPreferences.CleanupStep.class) : EnumSet.copyOf(allJobs); + selectedJobs = (selectedJobs == null) ? EnumSet.noneOf(CleanupPreferences.CleanupStep.class) : EnumSet.copyOf(selectedJobs); + formatters = Objects.requireNonNullElse(formatters, Optional.empty()); + } + + public static CleanupTabSelection ofJobs(EnumSet allJobs, EnumSet selectedJobs) { + return new CleanupTabSelection(allJobs, selectedJobs, Optional.empty()); + } + + public static CleanupTabSelection ofFormatters(FieldFormatterCleanups cleanups) { + return new CleanupTabSelection(EnumSet.noneOf(CleanupPreferences.CleanupStep.class), EnumSet.noneOf(CleanupPreferences.CleanupStep.class), Optional.of(cleanups)); + } +} diff --git a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupDialog.fxml b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupDialog.fxml index 85ee169dab3..9243c5ee63e 100644 --- a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupDialog.fxml +++ b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupDialog.fxml @@ -8,6 +8,5 @@ - diff --git a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml index efba4559d13..5c5dafc2b01 100644 --- a/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml +++ b/jabgui/src/main/resources/org/jabref/gui/cleanup/CleanupFileRelatedPanel.fxml @@ -1,6 +1,7 @@ + @@ -29,4 +30,7 @@ + +