Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhanced message log #4815

Merged
merged 13 commits into from
Apr 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions src/main/java/org/jabref/gui/BasePanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ public JabRefFrame frame() {
}

public void output(String s) {
frame.output(s);
dialogService.notify(s);
}

private void setupActions() {
Expand Down Expand Up @@ -441,7 +441,7 @@ private void delete(boolean cut, List<BibEntry> 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();
Expand Down Expand Up @@ -569,13 +569,12 @@ private void copyKeyAndTitle() {
}

private void openExternalFile() {
final List<BibEntry> 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<BibEntry> 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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
21 changes: 19 additions & 2 deletions src/main/java/org/jabref/gui/FXDialogService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}.
Expand All @@ -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) {
Expand Down Expand Up @@ -253,7 +269,8 @@ public <V> void showProgressDialogAndWait(String title, String content, Task<V>

@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
Expand Down
23 changes: 4 additions & 19 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);

Expand All @@ -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();
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
});
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/actions/CleanupAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -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())));
}

Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public void action() throws Exception {
List<BibEntry> 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;
}

Expand All @@ -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)));
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/actions/CopyDoiUrlAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ public void execute() {
Optional<String> 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));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 8 additions & 5 deletions src/main/java/org/jabref/gui/actions/LookupIdentifierAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -39,7 +40,7 @@ public LookupIdentifierAction(JabRefFrame frame, IdFetcher<T> 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);
Expand Down Expand Up @@ -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<T> identifier = Optional.empty();
try {
identifier = fetcher.findIdentifier(bibEntry);
Expand All @@ -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));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lookupIdentifiers is executed in a background thread and in order to provide feedback to the user, i used a workaround here and explicitly called DefaultTaskExecutor.runInJavaFXThread
However, i would suggest to completely remove the notifications here, because they do not provide much value

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
}
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/actions/WriteXMPAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)));
}

Expand Down
20 changes: 6 additions & 14 deletions src/main/java/org/jabref/gui/desktop/JabRefDesktop.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -239,19 +233,17 @@ 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 {
new ProcessBuilder(subcommands).start();
} 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)));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just saw that we seemed to forgot to convert that old swing dialog here. Could you please fix this as well?
dialogService.showErrorDialog or so should work
Thanks!

Copy link
Contributor Author

@r0light r0light Apr 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did it in f2a31d0, there was also another one in the same class

}
}
Expand Down
Loading