From a273eb1fa13317d4091a767849c40e9495de2b7e Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 3 Apr 2019 11:19:03 +0200 Subject: [PATCH] Enhanced message log (#4815) * refactored JabRefFrame.output and FXDialogService.notify to only use FXDialogService.notify * added logging to notifications and fixed problem with logger in JabRefGUI * moved copying of logevents to GuiAppender * directly use dialogService in BasePanel * changed constructor to avoid NPE * refactored code so that DialogService.notify does not need DefaultTaskExecutor.runInJavaFXThread * replaced old swing dialog * replaced another swing dialog --- src/main/java/org/jabref/gui/BasePanel.java | 23 +++++---- .../java/org/jabref/gui/FXDialogService.java | 21 ++++++++- src/main/java/org/jabref/gui/JabRefFrame.java | 23 ++------- .../org/jabref/gui/actions/CleanupAction.java | 4 +- .../actions/CopyBibTeXKeyAndLinkAction.java | 6 +-- .../jabref/gui/actions/CopyDoiUrlAction.java | 4 +- .../gui/actions/GenerateBibtexKeyAction.java | 4 +- .../gui/actions/LookupIdentifierAction.java | 13 +++-- .../jabref/gui/actions/NewDatabaseAction.java | 2 +- .../jabref/gui/actions/WriteXMPAction.java | 4 +- .../org/jabref/gui/desktop/JabRefDesktop.java | 20 +++----- .../jabref/gui/exporter/ExportCommand.java | 4 +- .../gui/exporter/ExportToClipboardAction.java | 4 +- .../gui/exporter/SaveDatabaseAction.java | 6 +-- .../gui/externalfiles/FindFullTextAction.java | 27 ++++------- .../ExternalFileMenuItem.java | 4 +- .../jabref/gui/filelist/AttachFileAction.java | 2 +- .../org/jabref/gui/importer/ImportAction.java | 5 +- .../actions/AppendDatabaseAction.java | 47 +++++++++---------- .../importer/actions/OpenDatabaseAction.java | 8 ++-- .../fetcher/WebSearchPaneViewModel.java | 4 +- .../jabref/gui/journals/AbbreviateAction.java | 3 +- .../gui/journals/UnabbreviateAction.java | 3 +- .../org/jabref/gui/logging/GuiAppender.java | 6 ++- .../gui/shared/SharedDatabaseUIManager.java | 11 ++--- .../gui/specialfields/SpecialFieldAction.java | 2 +- .../jabref/gui/worker/SendAsEMailAction.java | 4 +- .../org/jabref/logic/logging/LogMessages.java | 6 +-- 28 files changed, 126 insertions(+), 144 deletions(-) diff --git a/src/main/java/org/jabref/gui/BasePanel.java b/src/main/java/org/jabref/gui/BasePanel.java index 2ccfcdcc927..c6501ec7da9 100644 --- a/src/main/java/org/jabref/gui/BasePanel.java +++ b/src/main/java/org/jabref/gui/BasePanel.java @@ -250,7 +250,7 @@ public JabRefFrame frame() { } public void output(String s) { - frame.output(s); + dialogService.notify(s); } private void setupActions() { @@ -441,7 +441,7 @@ private void delete(boolean cut, List entries) { getUndoManager().addEdit(compound); markBaseChanged(); - frame.output(formatOutputMessage(cut ? Localization.lang("Cut") : Localization.lang("Deleted"), entries.size())); + this.output(formatOutputMessage(cut ? Localization.lang("Cut") : Localization.lang("Deleted"), entries.size())); // prevent the main table from loosing focus mainTable.requestFocus(); @@ -569,13 +569,12 @@ private void copyKeyAndTitle() { } private void openExternalFile() { + final List selectedEntries = mainTable.getSelectedEntries(); + if (selectedEntries.size() != 1) { + output(Localization.lang("This operation requires exactly one item to be selected.")); + return; + } JabRefExecutorService.INSTANCE.execute(() -> { - final List selectedEntries = mainTable.getSelectedEntries(); - if (selectedEntries.size() != 1) { - output(Localization.lang("This operation requires exactly one item to be selected.")); - return; - } - final BibEntry entry = selectedEntries.get(0); if (!entry.hasField(FieldName.FILE)) { // no bibtex field @@ -1292,10 +1291,10 @@ public void action() { try { getUndoManager().undo(); markBaseChanged(); - frame.output(Localization.lang("Undo")); + output(Localization.lang("Undo")); } catch (CannotUndoException ex) { LOGGER.warn("Nothing to undo", ex); - frame.output(Localization.lang("Nothing to undo") + '.'); + output(Localization.lang("Nothing to undo") + '.'); } markChangedOrUnChanged(); @@ -1363,9 +1362,9 @@ public void action() { try { getUndoManager().redo(); markBaseChanged(); - frame.output(Localization.lang("Redo")); + output(Localization.lang("Redo")); } catch (CannotRedoException ex) { - frame.output(Localization.lang("Nothing to redo") + '.'); + output(Localization.lang("Nothing to redo") + '.'); } markChangedOrUnChanged(); diff --git a/src/main/java/org/jabref/gui/FXDialogService.java b/src/main/java/org/jabref/gui/FXDialogService.java index f3edaba275a..2fe537a30a1 100644 --- a/src/main/java/org/jabref/gui/FXDialogService.java +++ b/src/main/java/org/jabref/gui/FXDialogService.java @@ -24,21 +24,27 @@ import javafx.scene.control.ChoiceDialog; import javafx.scene.control.DialogPane; import javafx.scene.control.TextInputDialog; +import javafx.scene.layout.Pane; import javafx.scene.layout.Region; import javafx.stage.DirectoryChooser; import javafx.stage.FileChooser; import javafx.stage.Stage; import javafx.stage.Window; +import javafx.util.Duration; -import org.jabref.JabRefGUI; import org.jabref.gui.icon.IconTheme; import org.jabref.gui.util.DirectoryDialogConfiguration; import org.jabref.gui.util.FileDialogConfiguration; import org.jabref.gui.util.ZipFileChooser; import org.jabref.logic.l10n.Localization; +import com.jfoenix.controls.JFXSnackbar; +import com.jfoenix.controls.JFXSnackbar.SnackbarEvent; +import com.jfoenix.controls.JFXSnackbarLayout; import org.controlsfx.dialog.ExceptionDialog; import org.controlsfx.dialog.ProgressDialog; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * This class provides methods to create default @@ -51,7 +57,11 @@ */ public class FXDialogService implements DialogService { + private static final Duration TOAST_MESSAGE_DISPLAY_TIME = Duration.millis(3000); + private static final Logger LOGGER = LoggerFactory.getLogger(FXDialogService.class); + private final Window mainWindow; + private final JFXSnackbar statusLine; /** * @deprecated try not to initialize a new dialog service but reuse the one constructed in {@link org.jabref.gui.JabRefFrame}. @@ -63,6 +73,12 @@ public FXDialogService() { public FXDialogService(Window mainWindow) { this.mainWindow = mainWindow; + this.statusLine = new JFXSnackbar(); + } + + public FXDialogService(Window mainWindow, Pane mainPane) { + this.mainWindow = mainWindow; + this.statusLine = new JFXSnackbar(mainPane); } private static FXDialog createDialog(AlertType type, String title, String content) { @@ -253,7 +269,8 @@ public void showProgressDialogAndWait(String title, String content, Task @Override public void notify(String message) { - JabRefGUI.getMainFrame().output(message); + LOGGER.info(message); + statusLine.fireEvent(new SnackbarEvent(new JFXSnackbarLayout(message), TOAST_MESSAGE_DISPLAY_TIME, null)); } @Override diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index d32b48dd30f..71e75d1c54d 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -50,7 +50,6 @@ import javafx.scene.layout.Pane; import javafx.scene.layout.Priority; import javafx.stage.Stage; -import javafx.util.Duration; import org.jabref.Globals; import org.jabref.JabRefExecutorService; @@ -132,9 +131,6 @@ import org.jabref.preferences.LastFocusedTabPreferences; import com.google.common.eventbus.Subscribe; -import com.jfoenix.controls.JFXSnackbar; -import com.jfoenix.controls.JFXSnackbar.SnackbarEvent; -import com.jfoenix.controls.JFXSnackbarLayout; import org.eclipse.fx.ui.controls.tabpane.DndTabPane; import org.eclipse.fx.ui.controls.tabpane.DndTabPaneFactory; import org.fxmisc.easybind.EasyBind; @@ -151,12 +147,11 @@ public class JabRefFrame extends BorderPane { public static final String FRAME_TITLE = "JabRef"; private static final Logger LOGGER = LoggerFactory.getLogger(JabRefFrame.class); - private static final Duration TOAST_MESSAGE_DISPLAY_TIME = Duration.millis(3000); private final SplitPane splitPane = new SplitPane(); private final JabRefPreferences prefs = Globals.prefs; private final GlobalSearchBar globalSearchBar = new GlobalSearchBar(this); - private final JFXSnackbar statusLine = new JFXSnackbar(this); + private final ProgressBar progressBar = new ProgressBar(); private final FileHistoryMenu fileHistory = new FileHistoryMenu(prefs, this); @@ -182,7 +177,7 @@ public class JabRefFrame extends BorderPane { public JabRefFrame(Stage mainStage) { this.mainStage = mainStage; - this.dialogService = new FXDialogService(mainStage); + this.dialogService = new FXDialogService(mainStage, this); init(); } @@ -962,16 +957,6 @@ public void addParserResult(ParserResult pr, boolean focusPanel) { } } - /** - * Displays the given message at the bottom of the main frame - * - * @deprecated use {@link DialogService#notify(String)} instead. However, do not remove this method, it's called from the dialogService - */ - @Deprecated - public void output(final String message) { - DefaultTaskExecutor.runInJavaFXThread(() -> statusLine.fireEvent(new SnackbarEvent(new JFXSnackbarLayout(message), TOAST_MESSAGE_DISPLAY_TIME, null))); - } - private void initActions() { /* openDatabaseOnlyActions.clear(); @@ -1279,7 +1264,7 @@ private boolean confirmClose(BasePanel panel) { return true; } // The action was either canceled or unsuccessful. - output(Localization.lang("Unable to save library")); + dialogService.notify(Localization.lang("Unable to save library")); } catch (Throwable ex) { LOGGER.error("A problem occurred when trying to save the file", ex); dialogService.showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex); @@ -1321,7 +1306,7 @@ private void removeTab(BasePanel panel) { panel.cleanUp(); tabbedPane.getTabs().remove(getTab(panel)); setWindowTitle(); - output(Localization.lang("Closed library") + '.'); + dialogService.notify(Localization.lang("Closed library") + '.'); // update tab titles updateAllTabTitles(); }); diff --git a/src/main/java/org/jabref/gui/actions/CleanupAction.java b/src/main/java/org/jabref/gui/actions/CleanupAction.java index f2251d45616..d79a8a62c4f 100644 --- a/src/main/java/org/jabref/gui/actions/CleanupAction.java +++ b/src/main/java/org/jabref/gui/actions/CleanupAction.java @@ -71,7 +71,7 @@ public void init() { isCanceled = true; return; } - panel.output(Localization.lang("Doing a cleanup for %0 entries...", + dialogService.notify(Localization.lang("Doing a cleanup for %0 entries...", Integer.toString(panel.getSelectedEntries().size()))); } @@ -115,7 +115,7 @@ private void showResults() { message = Localization.lang("%0 entries needed a clean up", Integer.toString(modifiedEntriesCount)); break; } - panel.output(message); + dialogService.notify(message); } private void cleanup(CleanupPreset cleanupPreset) { diff --git a/src/main/java/org/jabref/gui/actions/CopyBibTeXKeyAndLinkAction.java b/src/main/java/org/jabref/gui/actions/CopyBibTeXKeyAndLinkAction.java index 6f55af1e27b..49c8a5f46ec 100644 --- a/src/main/java/org/jabref/gui/actions/CopyBibTeXKeyAndLinkAction.java +++ b/src/main/java/org/jabref/gui/actions/CopyBibTeXKeyAndLinkAction.java @@ -36,7 +36,7 @@ public void action() throws Exception { List entriesWithKey = entries.stream().filter(BibEntry::hasCiteKey).collect(Collectors.toList()); if (entriesWithKey.isEmpty()) { - JabRefGUI.getMainFrame().output(Localization.lang("None of the selected entries have BibTeX keys.")); + JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("None of the selected entries have BibTeX keys.")); return; } @@ -53,9 +53,9 @@ public void action() throws Exception { int toCopy = entries.size(); if (copied == toCopy) { // All entries had keys. - JabRefGUI.getMainFrame().output((entries.size() > 1 ? Localization.lang("Copied keys") : Localization.lang("Copied key")) + '.'); + JabRefGUI.getMainFrame().getDialogService().notify((entries.size() > 1 ? Localization.lang("Copied keys") : Localization.lang("Copied key")) + '.'); } else { - JabRefGUI.getMainFrame().output(Localization.lang("Warning: %0 out of %1 entries have undefined BibTeX key.", + JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Warning: %0 out of %1 entries have undefined BibTeX key.", Long.toString(toCopy - copied), Integer.toString(toCopy))); } } diff --git a/src/main/java/org/jabref/gui/actions/CopyDoiUrlAction.java b/src/main/java/org/jabref/gui/actions/CopyDoiUrlAction.java index 51adf9ba8b3..e541cd58d5a 100644 --- a/src/main/java/org/jabref/gui/actions/CopyDoiUrlAction.java +++ b/src/main/java/org/jabref/gui/actions/CopyDoiUrlAction.java @@ -27,9 +27,9 @@ public void execute() { Optional urlOptional = DOI.parse(identifier).map(DOI::getURIAsASCIIString); if (urlOptional.isPresent()) { Globals.clipboardManager.setContent(urlOptional.get()); - JabRefGUI.getMainFrame().output(Localization.lang("The link has been copied to the clipboard.")); + JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("The link has been copied to the clipboard.")); } else { - JabRefGUI.getMainFrame().output(Localization.lang("Invalid DOI: '%0'.", identifier)); + JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Invalid DOI: '%0'.", identifier)); } } } diff --git a/src/main/java/org/jabref/gui/actions/GenerateBibtexKeyAction.java b/src/main/java/org/jabref/gui/actions/GenerateBibtexKeyAction.java index 90b61800215..744db49711c 100644 --- a/src/main/java/org/jabref/gui/actions/GenerateBibtexKeyAction.java +++ b/src/main/java/org/jabref/gui/actions/GenerateBibtexKeyAction.java @@ -33,7 +33,7 @@ public void init() { Localization.lang("First select the entries you want keys to be generated for.")); return; } - basePanel.output(formatOutputMessage(Localization.lang("Generating BibTeX key for"), entries.size())); + dialogService.notify(formatOutputMessage(Localization.lang("Generating BibTeX key for"), entries.size())); } public static boolean confirmOverwriteKeys(DialogService dialogService) { @@ -87,7 +87,7 @@ private void generateKeys() { } basePanel.markBaseChanged(); - basePanel.output(formatOutputMessage(Localization.lang("Generated BibTeX key for"), entries.size())); + dialogService.notify(formatOutputMessage(Localization.lang("Generated BibTeX key for"), entries.size())); } private String formatOutputMessage(String start, int count) { diff --git a/src/main/java/org/jabref/gui/actions/LookupIdentifierAction.java b/src/main/java/org/jabref/gui/actions/LookupIdentifierAction.java index f1f6e6f0846..a2d6c7372d9 100644 --- a/src/main/java/org/jabref/gui/actions/LookupIdentifierAction.java +++ b/src/main/java/org/jabref/gui/actions/LookupIdentifierAction.java @@ -12,6 +12,7 @@ import org.jabref.gui.undo.NamedCompound; import org.jabref.gui.undo.UndoableFieldChange; import org.jabref.gui.util.BackgroundTask; +import org.jabref.gui.util.DefaultTaskExecutor; import org.jabref.logic.importer.FetcherException; import org.jabref.logic.importer.IdFetcher; import org.jabref.logic.l10n.Localization; @@ -39,7 +40,7 @@ public LookupIdentifierAction(JabRefFrame frame, IdFetcher fetcher) { public void execute() { try { BackgroundTask.wrap(this::lookupIdentifiers) - .onSuccess(frame::output) + .onSuccess(frame.getDialogService()::notify) .executeWith(Globals.TASK_EXECUTOR); } catch (Exception e) { LOGGER.error("Problem running ID Worker", e); @@ -84,8 +85,9 @@ private String lookupIdentifiers() { int foundCount = 0; for (BibEntry bibEntry : bibEntries) { count++; - frame.output(Localization.lang("Looking up %0... - entry %1 out of %2 - found %3", - fetcher.getIdentifierName(), Integer.toString(count), totalCount, Integer.toString(foundCount))); + final String statusMessage = Localization.lang("Looking up %0... - entry %1 out of %2 - found %3", + fetcher.getIdentifierName(), Integer.toString(count), totalCount, Integer.toString(foundCount)); + DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().notify(statusMessage)); Optional identifier = Optional.empty(); try { identifier = fetcher.findIdentifier(bibEntry); @@ -97,8 +99,9 @@ private String lookupIdentifiers() { if (fieldChange.isPresent()) { namedCompound.addEdit(new UndoableFieldChange(fieldChange.get())); foundCount++; - frame.output(Localization.lang("Looking up %0... - entry %1 out of %2 - found %3", - Integer.toString(count), totalCount, Integer.toString(foundCount))); + final String nextStatusMessage = Localization.lang("Looking up %0... - entry %1 out of %2 - found %3", + fetcher.getIdentifierName(), Integer.toString(count), totalCount, Integer.toString(foundCount)); + DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().notify(nextStatusMessage)); } } } diff --git a/src/main/java/org/jabref/gui/actions/NewDatabaseAction.java b/src/main/java/org/jabref/gui/actions/NewDatabaseAction.java index 90b2f8ced30..e00604e3a1b 100644 --- a/src/main/java/org/jabref/gui/actions/NewDatabaseAction.java +++ b/src/main/java/org/jabref/gui/actions/NewDatabaseAction.java @@ -24,6 +24,6 @@ public void execute() { BibDatabaseContext bibDatabaseContext = new BibDatabaseContext(new Defaults(BibDatabaseMode.BIBTEX)); bibDatabaseContext.setMode(mode); jabRefFrame.addTab(bibDatabaseContext, true); - jabRefFrame.output(Localization.lang("New %0 library created.", mode.getFormattedName())); + jabRefFrame.getDialogService().notify(Localization.lang("New %0 library created.", mode.getFormattedName())); } } diff --git a/src/main/java/org/jabref/gui/actions/WriteXMPAction.java b/src/main/java/org/jabref/gui/actions/WriteXMPAction.java index de4face8b53..67a1217a113 100644 --- a/src/main/java/org/jabref/gui/actions/WriteXMPAction.java +++ b/src/main/java/org/jabref/gui/actions/WriteXMPAction.java @@ -92,7 +92,7 @@ public void init() { } optionsDialog.open(); - basePanel.output(Localization.lang("Writing XMP-metadata...")); + dialogService.notify(Localization.lang("Writing XMP-metadata...")); } private void writeXMP() { @@ -162,7 +162,7 @@ private void writeXMP() { return; } - basePanel.output(Localization.lang("Finished writing XMP for %0 file (%1 skipped, %2 errors).", + dialogService.notify(Localization.lang("Finished writing XMP for %0 file (%1 skipped, %2 errors).", String.valueOf(entriesChanged), String.valueOf(skipped), String.valueOf(errors))); } diff --git a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java index aa7f9dd22fc..fbd9478789b 100644 --- a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java +++ b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java @@ -10,8 +10,6 @@ import java.util.Optional; import java.util.regex.Pattern; -import javax.swing.JOptionPane; - import org.jabref.Globals; import org.jabref.JabRefGUI; import org.jabref.gui.desktop.os.DefaultDesktop; @@ -201,12 +199,8 @@ public static void openBrowserShowPopup(String url) { String couldNotOpenBrowser = Localization.lang("Could not open browser."); String openManually = Localization.lang("Please open %0 manually.", url); String copiedToClipboard = Localization.lang("The link has been copied to the clipboard."); - JabRefGUI.getMainFrame().output(couldNotOpenBrowser); - JOptionPane.showMessageDialog(null, - couldNotOpenBrowser + "\n" + openManually + "\n" + - copiedToClipboard, - couldNotOpenBrowser, - JOptionPane.ERROR_MESSAGE); + JabRefGUI.getMainFrame().getDialogService().notify(couldNotOpenBrowser); + JabRefGUI.getMainFrame().getDialogService().showErrorDialogAndWait(couldNotOpenBrowser, couldNotOpenBrowser + "\n" + openManually + "\n" + copiedToClipboard); } } @@ -239,7 +233,7 @@ public static void openConsole(File file) throws IOException { // replace the placeholder if used String commandLoggingText = command.replace("%DIR", absolutePath); - JabRefGUI.getMainFrame().output(Localization.lang("Executing command \"%0\"...", commandLoggingText)); + JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Executing command \"%0\"...", commandLoggingText)); LOGGER.info("Executing command \"" + commandLoggingText + "\"..."); try { @@ -247,11 +241,9 @@ public static void openConsole(File file) throws IOException { } catch (IOException exception) { LOGGER.error("Open console", exception); - JOptionPane.showMessageDialog(null, - Localization.lang("Error occured while executing the command \"%0\".", commandLoggingText), - Localization.lang("Open console") + " - " + Localization.lang("Error"), - JOptionPane.ERROR_MESSAGE); - JabRefGUI.getMainFrame().output(null); + JabRefGUI.getMainFrame().getDialogService().showErrorDialogAndWait( + Localization.lang("Open console") + " - " + Localization.lang("Error", + Localization.lang("Error occured while executing the command \"%0\".", commandLoggingText))); } } } diff --git a/src/main/java/org/jabref/gui/exporter/ExportCommand.java b/src/main/java/org/jabref/gui/exporter/ExportCommand.java index 36bb095a027..77b3a8430de 100644 --- a/src/main/java/org/jabref/gui/exporter/ExportCommand.java +++ b/src/main/java/org/jabref/gui/exporter/ExportCommand.java @@ -113,14 +113,14 @@ private void export(Path file, FileChooser.ExtensionFilter selectedExtensionFilt finEntries); return null; // can not use BackgroundTask.wrap(Runnable) because Runnable.run() can't throw Exceptions }) - .onSuccess(x -> frame.output(Localization.lang("%0 export successful", format.getName()))) + .onSuccess(x -> frame.getDialogService().notify(Localization.lang("%0 export successful", format.getName()))) .onFailure(this::handleError) .executeWith(Globals.TASK_EXECUTOR); } private void handleError(Exception ex) { LOGGER.warn("Problem exporting", ex); - frame.output(Localization.lang("Could not save file.")); + frame.getDialogService().notify(Localization.lang("Could not save file.")); // Need to warn the user that saving failed! frame.getDialogService().showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex); } diff --git a/src/main/java/org/jabref/gui/exporter/ExportToClipboardAction.java b/src/main/java/org/jabref/gui/exporter/ExportToClipboardAction.java index 2148a0146c0..122b5baf610 100644 --- a/src/main/java/org/jabref/gui/exporter/ExportToClipboardAction.java +++ b/src/main/java/org/jabref/gui/exporter/ExportToClipboardAction.java @@ -53,7 +53,7 @@ public void execute() { } if (panel.getSelectedEntries().isEmpty()) { - panel.output(Localization.lang("This operation requires one or more entries to be selected.")); + dialogService.notify(Localization.lang("This operation requires one or more entries to be selected.")); return; } @@ -123,7 +123,7 @@ private void setContentToClipboard(String content) { clipboardContent.putRtf(content); Globals.clipboardManager.setContent(clipboardContent); - panel.output(Localization.lang("Entries exported to clipboard") + ": " + entries.size()); + dialogService.notify(Localization.lang("Entries exported to clipboard") + ": " + entries.size()); } diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index b743b7fa199..db4a591b1ec 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -140,7 +140,7 @@ private boolean doSave() { // Reset title of tab frame.setTabTitle(panel, panel.getTabTitle(), panel.getBibDatabaseContext().getDatabaseFile().get().getAbsolutePath()); - frame.output(Localization.lang("Saved library") + " '" + frame.getDialogService().notify(Localization.lang("Saved library") + " '" + panel.getBibDatabaseContext().getDatabaseFile().get().getPath() + "'."); frame.setWindowTitle(); frame.updateAllTabTitles(); @@ -158,7 +158,7 @@ private boolean doSave() { public boolean save() { if (panel.getBibDatabaseContext().getDatabasePath().isPresent()) { - panel.frame().output(Localization.lang("Saving library") + "..."); + panel.frame().getDialogService().notify(Localization.lang("Saving library") + "..."); panel.setSaving(true); return doSave(); } else { @@ -240,7 +240,7 @@ public void saveSelectedAsPlain() { try { saveDatabase(path, true, prefs.getDefaultEncoding(), SavePreferences.DatabaseSaveType.PLAIN_BIBTEX); frame.getFileHistory().newFile(path); - frame.output(Localization.lang("Saved selected to '%0'.", path.toString())); + frame.getDialogService().notify(Localization.lang("Saved selected to '%0'.", path.toString())); } catch (SaveException ex) { LOGGER.error("A problem occurred when trying to save the file", ex); frame.getDialogService().showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex); diff --git a/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java b/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java index 3874a503d16..adb6affcc4c 100644 --- a/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java +++ b/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java @@ -2,8 +2,6 @@ import java.net.URL; import java.nio.file.Path; -import java.util.ArrayList; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; @@ -42,12 +40,6 @@ public FindFullTextAction(BasePanel basePanel) { @Override public void execute() { - BackgroundTask.wrap(this::findFullTexts) - .onSuccess(this::downloadFullTexts) - .executeWith(Globals.TASK_EXECUTOR); - } - - private Map, BibEntry> findFullTexts() { if (!basePanel.getSelectedEntries().isEmpty()) { basePanel.output(Localization.lang("Looking for full text document...")); } else { @@ -68,21 +60,24 @@ private Map, BibEntry> findFullTexts() { if (!confirmDownload) { basePanel.output(Localization.lang("Operation canceled.")); - return null; + return; } } + BackgroundTask.wrap(this::findFullTexts) + .onSuccess(this::downloadFullTexts) + .executeWith(Globals.TASK_EXECUTOR); + } + private Map, BibEntry> findFullTexts() { Map, BibEntry> downloads = new ConcurrentHashMap<>(); for (BibEntry entry : basePanel.getSelectedEntries()) { FulltextFetchers fetchers = new FulltextFetchers(Globals.prefs.getImportFormatPreferences()); downloads.put(fetchers.findFullTextPDF(entry), entry); } - return downloads; } private void downloadFullTexts(Map, BibEntry> downloads) { - List> finishedTasks = new ArrayList<>(); for (Map.Entry, BibEntry> download : downloads.entrySet()) { BibEntry entry = download.getValue(); Optional result = download.getKey(); @@ -94,20 +89,15 @@ private void downloadFullTexts(Map, BibEntry> downloads) { dialogService.showErrorDialogAndWait(Localization.lang("Directory not found"), Localization.lang("Main file directory not set!") + " " + Localization.lang("Preferences") + " -> " + Localization.lang("File")); - return; } - - //Download full text + //Download and link full text addLinkedFileFromURL(result.get(), entry); + } else { dialogService.notify(Localization.lang("No full text document found for entry %0.", entry.getCiteKeyOptional().orElse(Localization.lang("undefined")))); } - finishedTasks.add(result); - } - for (Optional result : finishedTasks) { - downloads.remove(result); } } @@ -119,7 +109,6 @@ private void downloadFullTexts(Map, BibEntry> downloads) { * @param entry the entry "value" */ private void addLinkedFileFromURL(URL url, BibEntry entry) { - LinkedFile newLinkedFile = new LinkedFile(url, ""); if (!entry.getFiles().contains(newLinkedFile)) { diff --git a/src/main/java/org/jabref/gui/externalfiletype/ExternalFileMenuItem.java b/src/main/java/org/jabref/gui/externalfiletype/ExternalFileMenuItem.java index a2ebcb02dae..e204ca6b761 100644 --- a/src/main/java/org/jabref/gui/externalfiletype/ExternalFileMenuItem.java +++ b/src/main/java/org/jabref/gui/externalfiletype/ExternalFileMenuItem.java @@ -58,12 +58,12 @@ public void actionPerformed(ActionEvent e) { boolean success = openLink(); if (!success) { List searchedDirs = databaseContext.getFileDirectoriesAsPaths(Globals.prefs.getFilePreferences()); - frame.output(Localization.lang("Unable to open %0", link) + " " + Arrays.toString(searchedDirs.toArray())); + frame.getDialogService().notify(Localization.lang("Unable to open %0", link) + " " + Arrays.toString(searchedDirs.toArray())); } } private boolean openLink() { - frame.output(Localization.lang("External viewer called") + "."); + frame.getDialogService().notify(Localization.lang("External viewer called") + "."); try { Optional type = fileType; if (!this.fileType.isPresent()) { diff --git a/src/main/java/org/jabref/gui/filelist/AttachFileAction.java b/src/main/java/org/jabref/gui/filelist/AttachFileAction.java index c36318ea60e..8ff43c37060 100644 --- a/src/main/java/org/jabref/gui/filelist/AttachFileAction.java +++ b/src/main/java/org/jabref/gui/filelist/AttachFileAction.java @@ -28,7 +28,7 @@ public AttachFileAction(BasePanel panel, DialogService dialogService) { @Override public void execute() { if (panel.getSelectedEntries().size() != 1) { - panel.output(Localization.lang("This operation requires exactly one item to be selected.")); + dialogService.notify(Localization.lang("This operation requires exactly one item to be selected.")); return; } BibEntry entry = panel.getSelectedEntries().get(0); diff --git a/src/main/java/org/jabref/gui/importer/ImportAction.java b/src/main/java/org/jabref/gui/importer/ImportAction.java index 99fa2b36f33..7a945fef1f8 100644 --- a/src/main/java/org/jabref/gui/importer/ImportAction.java +++ b/src/main/java/org/jabref/gui/importer/ImportAction.java @@ -15,6 +15,7 @@ import org.jabref.gui.DialogService; import org.jabref.gui.JabRefFrame; import org.jabref.gui.util.BackgroundTask; +import org.jabref.gui.util.DefaultTaskExecutor; import org.jabref.gui.util.TaskExecutor; import org.jabref.logic.importer.ImportException; import org.jabref.logic.importer.ImportFormatReader; @@ -95,11 +96,11 @@ private List doImport(List files) try { if (!importer.isPresent()) { // Unknown format: - frame.output(Localization.lang("Importing in unknown format") + "..."); + DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().notify(Localization.lang("Importing in unknown format") + "...")); // This import method never throws an IOException: imports.add(Globals.IMPORT_FORMAT_READER.importUnknownFormat(filename, Globals.getFileUpdateMonitor())); } else { - frame.output(Localization.lang("Importing in %0 format", importer.get().getName()) + "..."); + DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().notify(Localization.lang("Importing in %0 format", importer.get().getName()) + "...")); // Specific importer: ParserResult pr = importer.get().importDatabase(filename, Globals.prefs.getDefaultEncoding()); imports.add(new ImportFormatReader.UnknownFormatImport(importer.get().getName(), pr)); diff --git a/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java index 5231d0edede..a5f190f5de5 100644 --- a/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java @@ -9,7 +9,6 @@ import javax.swing.undo.CompoundEdit; import org.jabref.Globals; -import org.jabref.JabRefExecutorService; import org.jabref.gui.BasePanel; import org.jabref.gui.DialogService; import org.jabref.gui.JabRefFrame; @@ -18,6 +17,7 @@ import org.jabref.gui.undo.NamedCompound; import org.jabref.gui.undo.UndoableInsertEntry; import org.jabref.gui.undo.UndoableInsertString; +import org.jabref.gui.util.BackgroundTask; import org.jabref.gui.util.FileDialogConfiguration; import org.jabref.logic.importer.OpenDatabase; import org.jabref.logic.importer.ParserResult; @@ -159,32 +159,31 @@ public void action() { } filesToOpen.addAll(chosen); - // Run the actual open in a thread to prevent the program locking until the file is loaded. - JabRefExecutorService.INSTANCE.execute( - () -> openIt(dialog.importEntries(), dialog.importStrings(), dialog.importGroups(), dialog.importSelectorWords())); - } - } + if (filesToOpen.isEmpty()) { + return; + } - private void openIt(boolean importEntries, boolean importStrings, boolean importGroups, - boolean importSelectorWords) { - if (filesToOpen.isEmpty()) { - return; - } - for (Path file : filesToOpen) { - try { - Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, file.getParent().toString()); - // Should this be done _after_ we know it was successfully opened? - ParserResult parserResult = OpenDatabase.loadDatabase(file.toFile(), - Globals.prefs.getImportFormatPreferences(), Globals.getFileUpdateMonitor()); - AppendDatabaseAction.mergeFromBibtex(panel, parserResult, importEntries, importStrings, importGroups, - importSelectorWords); - panel.output(Localization.lang("Imported from library") + " '" + file + "'"); - } catch (IOException | KeyCollisionException ex) { - LOGGER.warn("Could not open database", ex); - - dialogService.showErrorDialogAndWait(Localization.lang("Open library"), ex); + for (Path file : filesToOpen) { + // Run the actual open in a thread to prevent the program locking until the file is loaded. + BackgroundTask.wrap(() -> openIt(file, dialog.importEntries(), dialog.importStrings(), dialog.importGroups(), dialog.importSelectorWords())) + .onSuccess(fileName -> dialogService.notify(Localization.lang("Imported from library") + " '" + fileName + "'")) + .onFailure(exception -> { + LOGGER.warn("Could not open database", exception); + dialogService.showErrorDialogAndWait(Localization.lang("Open library"), exception);}) + .executeWith(Globals.TASK_EXECUTOR);; } } } + private String openIt(Path file, boolean importEntries, boolean importStrings, boolean importGroups, + boolean importSelectorWords) throws IOException, KeyCollisionException { + Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, file.getParent().toString()); + // Should this be done _after_ we know it was successfully opened? + ParserResult parserResult = OpenDatabase.loadDatabase(file.toFile(), + Globals.prefs.getImportFormatPreferences(), Globals.getFileUpdateMonitor()); + AppendDatabaseAction.mergeFromBibtex(panel, parserResult, importEntries, importStrings, importGroups, + importSelectorWords); + return file.toString(); + } + } diff --git a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java index 67bd022cbc0..b8be7090041 100644 --- a/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java +++ b/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java @@ -174,12 +174,12 @@ public void openFiles(List filesToOpen, boolean raisePanel) { // If no files are remaining to open, this could mean that a file was // already open. If so, we may have to raise the correct tab: else if (toRaise != null) { - frame.output(Localization.lang("File '%0' is already open.", + frame.getDialogService().notify(Localization.lang("File '%0' is already open.", toRaise.getBibDatabaseContext().getDatabaseFile().get().getPath())); frame.showBasePanel(toRaise); } - frame.output(Localization.lang("Files opened") + ": " + (filesToOpen.size())); + frame.getDialogService().notify(Localization.lang("Files opened") + ": " + (filesToOpen.size())); } /** @@ -190,7 +190,7 @@ private void openTheFile(Path file, boolean raisePanel) { if (Files.exists(file)) { Path fileToLoad = file.toAbsolutePath(); - frame.output(Localization.lang("Opening") + ": '" + file + "'"); + frame.getDialogService().notify(Localization.lang("Opening") + ": '" + file + "'"); Globals.prefs.put(JabRefPreferences.WORKING_DIRECTORY, fileToLoad.getParent().toString()); @@ -239,7 +239,7 @@ private BasePanel addNewDatabase(ParserResult result, final Path file, boolean r } if (Objects.nonNull(file)) { - frame.output(Localization.lang("Opened library") + " '" + file.toString() + "' " + frame.getDialogService().notify(Localization.lang("Opened library") + " '" + file.toString() + "' " + Localization.lang("with") + " " + database.getEntryCount() + " " + Localization.lang("entries") + "."); diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java index 77354393115..dba5c3352da 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java @@ -84,12 +84,12 @@ public StringProperty queryProperty() { public void search() { if (StringUtil.isBlank(getQuery())) { - frame.output(Localization.lang("Please enter a search string")); + frame.getDialogService().notify(Localization.lang("Please enter a search string")); return; } if (frame.getCurrentBasePanel() == null) { - frame.output(Localization.lang("Please open or start a new library before searching")); + frame.getDialogService().notify(Localization.lang("Please open or start a new library before searching")); return; } diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java index 283905fb435..734074c74fe 100644 --- a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java @@ -37,6 +37,7 @@ public AbbreviateAction(BasePanel panel, boolean iso) { @Override public void action() { + panel.output(Localization.lang("Abbreviating...")); BackgroundTask.wrap(this::abbreviate) .onSuccess(panel::output) .executeWith(Globals.TASK_EXECUTOR); @@ -44,8 +45,6 @@ public void action() { } private String abbreviate() { - panel.output(Localization.lang("Abbreviating...")); - List entries = panel.getSelectedEntries(); UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator( Globals.journalAbbreviationLoader.getRepository(Globals.prefs.getJournalAbbreviationPreferences()), diff --git a/src/main/java/org/jabref/gui/journals/UnabbreviateAction.java b/src/main/java/org/jabref/gui/journals/UnabbreviateAction.java index 770c996856a..7980beabdc0 100644 --- a/src/main/java/org/jabref/gui/journals/UnabbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/UnabbreviateAction.java @@ -24,14 +24,13 @@ public UnabbreviateAction(BasePanel panel) { @Override public void action() { + panel.output(Localization.lang("Unabbreviating...")); BackgroundTask.wrap(this::unabbreviate) .onSuccess(panel::output) .executeWith(Globals.TASK_EXECUTOR); } private String unabbreviate() { - panel.output(Localization.lang("Unabbreviating...")); - List entries = panel.getSelectedEntries(); // never null UndoableUnabbreviator undoableAbbreviator = new UndoableUnabbreviator(Globals.journalAbbreviationLoader diff --git a/src/main/java/org/jabref/gui/logging/GuiAppender.java b/src/main/java/org/jabref/gui/logging/GuiAppender.java index d6a6f25ebd6..4d5956b94c5 100644 --- a/src/main/java/org/jabref/gui/logging/GuiAppender.java +++ b/src/main/java/org/jabref/gui/logging/GuiAppender.java @@ -13,6 +13,7 @@ import org.apache.logging.log4j.core.config.plugins.PluginAttribute; import org.apache.logging.log4j.core.config.plugins.PluginElement; import org.apache.logging.log4j.core.config.plugins.PluginFactory; +import org.apache.logging.log4j.core.impl.MutableLogEvent; import org.apache.logging.log4j.core.layout.PatternLayout; @Plugin(name = "GuiAppender", category = "Core", elementType = "appender", printObject = true) @@ -44,6 +45,9 @@ public static GuiAppender createAppender(@PluginAttribute("name") String name, */ @Override public void append(LogEvent event) { - DefaultTaskExecutor.runInJavaFXThread(() -> LogMessages.getInstance().add(event)); + // We need to make a copy as instances of LogEvent are reused by log4j + MutableLogEvent copy = new MutableLogEvent(); + copy.initFrom(event); + DefaultTaskExecutor.runInJavaFXThread(() -> LogMessages.getInstance().add(copy)); } } diff --git a/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java b/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java index 219c157fc53..94bc02ddc5b 100644 --- a/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java +++ b/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java @@ -48,7 +48,7 @@ public SharedDatabaseUIManager(JabRefFrame jabRefFrame) { @Subscribe public void listen(ConnectionLostEvent connectionLostEvent) { - jabRefFrame.output(Localization.lang("Connection lost.")); + jabRefFrame.getDialogService().notify(Localization.lang("Connection lost.")); ButtonType reconnect = new ButtonType(Localization.lang("Reconnect"), ButtonData.YES); ButtonType workOffline = new ButtonType(Localization.lang("Work offline"), ButtonData.NO); @@ -69,7 +69,7 @@ public void listen(ConnectionLostEvent connectionLostEvent) { } else if (answer.get().equals(workOffline)) { connectionLostEvent.getBibDatabaseContext().convertToLocalDatabase(); jabRefFrame.refreshTitleAndTabs(); - jabRefFrame.output(Localization.lang("Working offline.")); + jabRefFrame.getDialogService().notify(Localization.lang("Working offline.")); } } else { jabRefFrame.closeCurrentTab(); @@ -79,7 +79,7 @@ public void listen(ConnectionLostEvent connectionLostEvent) { @Subscribe public void listen(UpdateRefusedEvent updateRefusedEvent) { - jabRefFrame.output(Localization.lang("Update refused.")); + jabRefFrame.getDialogService().notify(Localization.lang("Update refused.")); new MergeSharedEntryDialog(jabRefFrame, dbmsSynchronizer, updateRefusedEvent.getLocalBibEntry(), updateRefusedEvent.getSharedBibEntry(), @@ -107,7 +107,6 @@ public void listen(SharedEntryNotPresentEvent event) { * Opens a new shared database tab with the given {@link DBMSConnectionProperties}. * * @param dbmsConnectionProperties Connection data - * @param raiseTab If true the new tab gets selected. * @return BasePanel which also used by {@link SaveDatabaseAction} */ public BasePanel openNewSharedDatabaseTab(DBMSConnectionProperties dbmsConnectionProperties) @@ -121,7 +120,7 @@ public BasePanel openNewSharedDatabaseTab(DBMSConnectionProperties dbmsConnectio dbmsSynchronizer = bibDatabaseContext.getDBMSSynchronizer(); dbmsSynchronizer.openSharedDatabase(new DBMSConnection(dbmsConnectionProperties)); dbmsSynchronizer.registerListener(this); - jabRefFrame.output(Localization.lang("Connection to %0 server established.", dbmsConnectionProperties.getType().toString())); + jabRefFrame.getDialogService().notify(Localization.lang("Connection to %0 server established.", dbmsConnectionProperties.getType().toString())); return jabRefFrame.addTab(bibDatabaseContext, true); } @@ -150,6 +149,6 @@ public void openSharedDatabaseFromParserResult(ParserResult parserResult) dbmsSynchronizer.openSharedDatabase(new DBMSConnection(dbmsConnectionProperties)); dbmsSynchronizer.registerListener(this); parserResult.setDatabaseContext(bibDatabaseContext); - jabRefFrame.output(Localization.lang("Connection to %0 server established.", dbmsConnectionProperties.getType().toString())); + jabRefFrame.getDialogService().notify(Localization.lang("Connection to %0 server established.", dbmsConnectionProperties.getType().toString())); } } diff --git a/src/main/java/org/jabref/gui/specialfields/SpecialFieldAction.java b/src/main/java/org/jabref/gui/specialfields/SpecialFieldAction.java index 7c5f1f0e217..db486945462 100644 --- a/src/main/java/org/jabref/gui/specialfields/SpecialFieldAction.java +++ b/src/main/java/org/jabref/gui/specialfields/SpecialFieldAction.java @@ -70,7 +70,7 @@ public void action() { } else { outText = getTextDone(specialField, value, Integer.toString(bes.size())); } - frame.output(outText); + frame.getDialogService().notify(outText); } else { // if user does not change anything with his action, we do not do anything either // even no output message diff --git a/src/main/java/org/jabref/gui/worker/SendAsEMailAction.java b/src/main/java/org/jabref/gui/worker/SendAsEMailAction.java index 51b85ba2784..ee8b7eace9a 100644 --- a/src/main/java/org/jabref/gui/worker/SendAsEMailAction.java +++ b/src/main/java/org/jabref/gui/worker/SendAsEMailAction.java @@ -46,11 +46,11 @@ public SendAsEMailAction(JabRefFrame frame) { @Override public void action() { BackgroundTask.wrap(this::sendEmail) - .onSuccess(frame::output) + .onSuccess(frame.getDialogService()::notify) .onFailure(e -> { String message = Localization.lang("Error creating email"); LOGGER.warn(message, e); - frame.output(message); + frame.getDialogService().notify(message); }) .executeWith(Globals.TASK_EXECUTOR); } diff --git a/src/main/java/org/jabref/logic/logging/LogMessages.java b/src/main/java/org/jabref/logic/logging/LogMessages.java index 06f10f1c3eb..053252b6e1f 100644 --- a/src/main/java/org/jabref/logic/logging/LogMessages.java +++ b/src/main/java/org/jabref/logic/logging/LogMessages.java @@ -4,7 +4,6 @@ import javafx.collections.ObservableList; import org.apache.logging.log4j.core.LogEvent; -import org.apache.logging.log4j.core.impl.MutableLogEvent; /** * This class is used for storing and archiving all message output of JabRef as log events. @@ -28,10 +27,7 @@ public ObservableList getMessages() { } public void add(LogEvent event) { - // We need to make a copy as instances of LogEvent are reused by log4j - MutableLogEvent copy = new MutableLogEvent(); - copy.initFrom(event); - messages.add(copy); + messages.add(event); } public void clear() {