Skip to content

Commit a273eb1

Browse files
r0lightSiedlerchr
authored andcommitted
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
1 parent 47772fd commit a273eb1

28 files changed

+126
-144
lines changed

src/main/java/org/jabref/gui/BasePanel.java

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ public JabRefFrame frame() {
250250
}
251251

252252
public void output(String s) {
253-
frame.output(s);
253+
dialogService.notify(s);
254254
}
255255

256256
private void setupActions() {
@@ -441,7 +441,7 @@ private void delete(boolean cut, List<BibEntry> entries) {
441441
getUndoManager().addEdit(compound);
442442

443443
markBaseChanged();
444-
frame.output(formatOutputMessage(cut ? Localization.lang("Cut") : Localization.lang("Deleted"), entries.size()));
444+
this.output(formatOutputMessage(cut ? Localization.lang("Cut") : Localization.lang("Deleted"), entries.size()));
445445

446446
// prevent the main table from loosing focus
447447
mainTable.requestFocus();
@@ -569,13 +569,12 @@ private void copyKeyAndTitle() {
569569
}
570570

571571
private void openExternalFile() {
572+
final List<BibEntry> selectedEntries = mainTable.getSelectedEntries();
573+
if (selectedEntries.size() != 1) {
574+
output(Localization.lang("This operation requires exactly one item to be selected."));
575+
return;
576+
}
572577
JabRefExecutorService.INSTANCE.execute(() -> {
573-
final List<BibEntry> selectedEntries = mainTable.getSelectedEntries();
574-
if (selectedEntries.size() != 1) {
575-
output(Localization.lang("This operation requires exactly one item to be selected."));
576-
return;
577-
}
578-
579578
final BibEntry entry = selectedEntries.get(0);
580579
if (!entry.hasField(FieldName.FILE)) {
581580
// no bibtex field
@@ -1292,10 +1291,10 @@ public void action() {
12921291
try {
12931292
getUndoManager().undo();
12941293
markBaseChanged();
1295-
frame.output(Localization.lang("Undo"));
1294+
output(Localization.lang("Undo"));
12961295
} catch (CannotUndoException ex) {
12971296
LOGGER.warn("Nothing to undo", ex);
1298-
frame.output(Localization.lang("Nothing to undo") + '.');
1297+
output(Localization.lang("Nothing to undo") + '.');
12991298
}
13001299

13011300
markChangedOrUnChanged();
@@ -1363,9 +1362,9 @@ public void action() {
13631362
try {
13641363
getUndoManager().redo();
13651364
markBaseChanged();
1366-
frame.output(Localization.lang("Redo"));
1365+
output(Localization.lang("Redo"));
13671366
} catch (CannotRedoException ex) {
1368-
frame.output(Localization.lang("Nothing to redo") + '.');
1367+
output(Localization.lang("Nothing to redo") + '.');
13691368
}
13701369

13711370
markChangedOrUnChanged();

src/main/java/org/jabref/gui/FXDialogService.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,27 @@
2424
import javafx.scene.control.ChoiceDialog;
2525
import javafx.scene.control.DialogPane;
2626
import javafx.scene.control.TextInputDialog;
27+
import javafx.scene.layout.Pane;
2728
import javafx.scene.layout.Region;
2829
import javafx.stage.DirectoryChooser;
2930
import javafx.stage.FileChooser;
3031
import javafx.stage.Stage;
3132
import javafx.stage.Window;
33+
import javafx.util.Duration;
3234

33-
import org.jabref.JabRefGUI;
3435
import org.jabref.gui.icon.IconTheme;
3536
import org.jabref.gui.util.DirectoryDialogConfiguration;
3637
import org.jabref.gui.util.FileDialogConfiguration;
3738
import org.jabref.gui.util.ZipFileChooser;
3839
import org.jabref.logic.l10n.Localization;
3940

41+
import com.jfoenix.controls.JFXSnackbar;
42+
import com.jfoenix.controls.JFXSnackbar.SnackbarEvent;
43+
import com.jfoenix.controls.JFXSnackbarLayout;
4044
import org.controlsfx.dialog.ExceptionDialog;
4145
import org.controlsfx.dialog.ProgressDialog;
46+
import org.slf4j.Logger;
47+
import org.slf4j.LoggerFactory;
4248

4349
/**
4450
* This class provides methods to create default
@@ -51,7 +57,11 @@
5157
*/
5258
public class FXDialogService implements DialogService {
5359

60+
private static final Duration TOAST_MESSAGE_DISPLAY_TIME = Duration.millis(3000);
61+
private static final Logger LOGGER = LoggerFactory.getLogger(FXDialogService.class);
62+
5463
private final Window mainWindow;
64+
private final JFXSnackbar statusLine;
5565

5666
/**
5767
* @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() {
6373

6474
public FXDialogService(Window mainWindow) {
6575
this.mainWindow = mainWindow;
76+
this.statusLine = new JFXSnackbar();
77+
}
78+
79+
public FXDialogService(Window mainWindow, Pane mainPane) {
80+
this.mainWindow = mainWindow;
81+
this.statusLine = new JFXSnackbar(mainPane);
6682
}
6783

6884
private static FXDialog createDialog(AlertType type, String title, String content) {
@@ -253,7 +269,8 @@ public <V> void showProgressDialogAndWait(String title, String content, Task<V>
253269

254270
@Override
255271
public void notify(String message) {
256-
JabRefGUI.getMainFrame().output(message);
272+
LOGGER.info(message);
273+
statusLine.fireEvent(new SnackbarEvent(new JFXSnackbarLayout(message), TOAST_MESSAGE_DISPLAY_TIME, null));
257274
}
258275

259276
@Override

src/main/java/org/jabref/gui/JabRefFrame.java

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
import javafx.scene.layout.Pane;
5151
import javafx.scene.layout.Priority;
5252
import javafx.stage.Stage;
53-
import javafx.util.Duration;
5453

5554
import org.jabref.Globals;
5655
import org.jabref.JabRefExecutorService;
@@ -132,9 +131,6 @@
132131
import org.jabref.preferences.LastFocusedTabPreferences;
133132

134133
import com.google.common.eventbus.Subscribe;
135-
import com.jfoenix.controls.JFXSnackbar;
136-
import com.jfoenix.controls.JFXSnackbar.SnackbarEvent;
137-
import com.jfoenix.controls.JFXSnackbarLayout;
138134
import org.eclipse.fx.ui.controls.tabpane.DndTabPane;
139135
import org.eclipse.fx.ui.controls.tabpane.DndTabPaneFactory;
140136
import org.fxmisc.easybind.EasyBind;
@@ -151,12 +147,11 @@ public class JabRefFrame extends BorderPane {
151147
public static final String FRAME_TITLE = "JabRef";
152148

153149
private static final Logger LOGGER = LoggerFactory.getLogger(JabRefFrame.class);
154-
private static final Duration TOAST_MESSAGE_DISPLAY_TIME = Duration.millis(3000);
155150

156151
private final SplitPane splitPane = new SplitPane();
157152
private final JabRefPreferences prefs = Globals.prefs;
158153
private final GlobalSearchBar globalSearchBar = new GlobalSearchBar(this);
159-
private final JFXSnackbar statusLine = new JFXSnackbar(this);
154+
160155
private final ProgressBar progressBar = new ProgressBar();
161156
private final FileHistoryMenu fileHistory = new FileHistoryMenu(prefs, this);
162157

@@ -182,7 +177,7 @@ public class JabRefFrame extends BorderPane {
182177

183178
public JabRefFrame(Stage mainStage) {
184179
this.mainStage = mainStage;
185-
this.dialogService = new FXDialogService(mainStage);
180+
this.dialogService = new FXDialogService(mainStage, this);
186181
init();
187182
}
188183

@@ -962,16 +957,6 @@ public void addParserResult(ParserResult pr, boolean focusPanel) {
962957
}
963958
}
964959

965-
/**
966-
* Displays the given message at the bottom of the main frame
967-
*
968-
* @deprecated use {@link DialogService#notify(String)} instead. However, do not remove this method, it's called from the dialogService
969-
*/
970-
@Deprecated
971-
public void output(final String message) {
972-
DefaultTaskExecutor.runInJavaFXThread(() -> statusLine.fireEvent(new SnackbarEvent(new JFXSnackbarLayout(message), TOAST_MESSAGE_DISPLAY_TIME, null)));
973-
}
974-
975960
private void initActions() {
976961
/*
977962
openDatabaseOnlyActions.clear();
@@ -1279,7 +1264,7 @@ private boolean confirmClose(BasePanel panel) {
12791264
return true;
12801265
}
12811266
// The action was either canceled or unsuccessful.
1282-
output(Localization.lang("Unable to save library"));
1267+
dialogService.notify(Localization.lang("Unable to save library"));
12831268
} catch (Throwable ex) {
12841269
LOGGER.error("A problem occurred when trying to save the file", ex);
12851270
dialogService.showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex);
@@ -1321,7 +1306,7 @@ private void removeTab(BasePanel panel) {
13211306
panel.cleanUp();
13221307
tabbedPane.getTabs().remove(getTab(panel));
13231308
setWindowTitle();
1324-
output(Localization.lang("Closed library") + '.');
1309+
dialogService.notify(Localization.lang("Closed library") + '.');
13251310
// update tab titles
13261311
updateAllTabTitles();
13271312
});

src/main/java/org/jabref/gui/actions/CleanupAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public void init() {
7171
isCanceled = true;
7272
return;
7373
}
74-
panel.output(Localization.lang("Doing a cleanup for %0 entries...",
74+
dialogService.notify(Localization.lang("Doing a cleanup for %0 entries...",
7575
Integer.toString(panel.getSelectedEntries().size())));
7676
}
7777

@@ -115,7 +115,7 @@ private void showResults() {
115115
message = Localization.lang("%0 entries needed a clean up", Integer.toString(modifiedEntriesCount));
116116
break;
117117
}
118-
panel.output(message);
118+
dialogService.notify(message);
119119
}
120120

121121
private void cleanup(CleanupPreset cleanupPreset) {

src/main/java/org/jabref/gui/actions/CopyBibTeXKeyAndLinkAction.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public void action() throws Exception {
3636
List<BibEntry> entriesWithKey = entries.stream().filter(BibEntry::hasCiteKey).collect(Collectors.toList());
3737

3838
if (entriesWithKey.isEmpty()) {
39-
JabRefGUI.getMainFrame().output(Localization.lang("None of the selected entries have BibTeX keys."));
39+
JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("None of the selected entries have BibTeX keys."));
4040
return;
4141
}
4242

@@ -53,9 +53,9 @@ public void action() throws Exception {
5353
int toCopy = entries.size();
5454
if (copied == toCopy) {
5555
// All entries had keys.
56-
JabRefGUI.getMainFrame().output((entries.size() > 1 ? Localization.lang("Copied keys") : Localization.lang("Copied key")) + '.');
56+
JabRefGUI.getMainFrame().getDialogService().notify((entries.size() > 1 ? Localization.lang("Copied keys") : Localization.lang("Copied key")) + '.');
5757
} else {
58-
JabRefGUI.getMainFrame().output(Localization.lang("Warning: %0 out of %1 entries have undefined BibTeX key.",
58+
JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Warning: %0 out of %1 entries have undefined BibTeX key.",
5959
Long.toString(toCopy - copied), Integer.toString(toCopy)));
6060
}
6161
}

src/main/java/org/jabref/gui/actions/CopyDoiUrlAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ public void execute() {
2727
Optional<String> urlOptional = DOI.parse(identifier).map(DOI::getURIAsASCIIString);
2828
if (urlOptional.isPresent()) {
2929
Globals.clipboardManager.setContent(urlOptional.get());
30-
JabRefGUI.getMainFrame().output(Localization.lang("The link has been copied to the clipboard."));
30+
JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("The link has been copied to the clipboard."));
3131
} else {
32-
JabRefGUI.getMainFrame().output(Localization.lang("Invalid DOI: '%0'.", identifier));
32+
JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Invalid DOI: '%0'.", identifier));
3333
}
3434
}
3535
}

src/main/java/org/jabref/gui/actions/GenerateBibtexKeyAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public void init() {
3333
Localization.lang("First select the entries you want keys to be generated for."));
3434
return;
3535
}
36-
basePanel.output(formatOutputMessage(Localization.lang("Generating BibTeX key for"), entries.size()));
36+
dialogService.notify(formatOutputMessage(Localization.lang("Generating BibTeX key for"), entries.size()));
3737
}
3838

3939
public static boolean confirmOverwriteKeys(DialogService dialogService) {
@@ -87,7 +87,7 @@ private void generateKeys() {
8787
}
8888

8989
basePanel.markBaseChanged();
90-
basePanel.output(formatOutputMessage(Localization.lang("Generated BibTeX key for"), entries.size()));
90+
dialogService.notify(formatOutputMessage(Localization.lang("Generated BibTeX key for"), entries.size()));
9191
}
9292

9393
private String formatOutputMessage(String start, int count) {

src/main/java/org/jabref/gui/actions/LookupIdentifierAction.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.jabref.gui.undo.NamedCompound;
1313
import org.jabref.gui.undo.UndoableFieldChange;
1414
import org.jabref.gui.util.BackgroundTask;
15+
import org.jabref.gui.util.DefaultTaskExecutor;
1516
import org.jabref.logic.importer.FetcherException;
1617
import org.jabref.logic.importer.IdFetcher;
1718
import org.jabref.logic.l10n.Localization;
@@ -39,7 +40,7 @@ public LookupIdentifierAction(JabRefFrame frame, IdFetcher<T> fetcher) {
3940
public void execute() {
4041
try {
4142
BackgroundTask.wrap(this::lookupIdentifiers)
42-
.onSuccess(frame::output)
43+
.onSuccess(frame.getDialogService()::notify)
4344
.executeWith(Globals.TASK_EXECUTOR);
4445
} catch (Exception e) {
4546
LOGGER.error("Problem running ID Worker", e);
@@ -84,8 +85,9 @@ private String lookupIdentifiers() {
8485
int foundCount = 0;
8586
for (BibEntry bibEntry : bibEntries) {
8687
count++;
87-
frame.output(Localization.lang("Looking up %0... - entry %1 out of %2 - found %3",
88-
fetcher.getIdentifierName(), Integer.toString(count), totalCount, Integer.toString(foundCount)));
88+
final String statusMessage = Localization.lang("Looking up %0... - entry %1 out of %2 - found %3",
89+
fetcher.getIdentifierName(), Integer.toString(count), totalCount, Integer.toString(foundCount));
90+
DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().notify(statusMessage));
8991
Optional<T> identifier = Optional.empty();
9092
try {
9193
identifier = fetcher.findIdentifier(bibEntry);
@@ -97,8 +99,9 @@ private String lookupIdentifiers() {
9799
if (fieldChange.isPresent()) {
98100
namedCompound.addEdit(new UndoableFieldChange(fieldChange.get()));
99101
foundCount++;
100-
frame.output(Localization.lang("Looking up %0... - entry %1 out of %2 - found %3",
101-
Integer.toString(count), totalCount, Integer.toString(foundCount)));
102+
final String nextStatusMessage = Localization.lang("Looking up %0... - entry %1 out of %2 - found %3",
103+
fetcher.getIdentifierName(), Integer.toString(count), totalCount, Integer.toString(foundCount));
104+
DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().notify(nextStatusMessage));
102105
}
103106
}
104107
}

src/main/java/org/jabref/gui/actions/NewDatabaseAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,6 @@ public void execute() {
2424
BibDatabaseContext bibDatabaseContext = new BibDatabaseContext(new Defaults(BibDatabaseMode.BIBTEX));
2525
bibDatabaseContext.setMode(mode);
2626
jabRefFrame.addTab(bibDatabaseContext, true);
27-
jabRefFrame.output(Localization.lang("New %0 library created.", mode.getFormattedName()));
27+
jabRefFrame.getDialogService().notify(Localization.lang("New %0 library created.", mode.getFormattedName()));
2828
}
2929
}

src/main/java/org/jabref/gui/actions/WriteXMPAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public void init() {
9292
}
9393
optionsDialog.open();
9494

95-
basePanel.output(Localization.lang("Writing XMP-metadata..."));
95+
dialogService.notify(Localization.lang("Writing XMP-metadata..."));
9696
}
9797

9898
private void writeXMP() {
@@ -162,7 +162,7 @@ private void writeXMP() {
162162
return;
163163
}
164164

165-
basePanel.output(Localization.lang("Finished writing XMP for %0 file (%1 skipped, %2 errors).",
165+
dialogService.notify(Localization.lang("Finished writing XMP for %0 file (%1 skipped, %2 errors).",
166166
String.valueOf(entriesChanged), String.valueOf(skipped), String.valueOf(errors)));
167167
}
168168

0 commit comments

Comments
 (0)