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 1 commit
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
11 changes: 5 additions & 6 deletions src/main/java/org/jabref/gui/BasePanel.java
Original file line number Diff line number Diff line change
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
3 changes: 1 addition & 2 deletions src/main/java/org/jabref/gui/FXDialogService.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import javafx.util.Duration;

import org.jabref.gui.icon.IconTheme;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.gui.util.DirectoryDialogConfiguration;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.gui.util.ZipFileChooser;
Expand Down Expand Up @@ -271,7 +270,7 @@ public <V> void showProgressDialogAndWait(String title, String content, Task<V>
@Override
public void notify(String message) {
LOGGER.info(message);
DefaultTaskExecutor.runInJavaFXThread(() -> statusLine.fireEvent(new SnackbarEvent(new JFXSnackbarLayout(message), TOAST_MESSAGE_DISPLAY_TIME, null)));
statusLine.fireEvent(new SnackbarEvent(new JFXSnackbarLayout(message), TOAST_MESSAGE_DISPLAY_TIME, null));
}

@Override
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 @@ -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
11 changes: 7 additions & 4 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 @@ -84,8 +85,9 @@ private String lookupIdentifiers() {
int foundCount = 0;
for (BibEntry bibEntry : bibEntries) {
count++;
frame.getDialogService().notify(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.getDialogService().notify(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
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
1 change: 0 additions & 1 deletion src/main/java/org/jabref/gui/desktop/JabRefDesktop.java
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ public static void openConsole(File file) throws IOException {
Localization.lang("Error occured while executing the command \"%0\".", commandLoggingText),
Localization.lang("Open console") + " - " + Localization.lang("Error"),
JOptionPane.ERROR_MESSAGE);
JabRefGUI.getMainFrame().getDialogService().notify(null);
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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());

}

Expand Down
27 changes: 8 additions & 19 deletions src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Optional<URL>, BibEntry> findFullTexts() {
if (!basePanel.getSelectedEntries().isEmpty()) {
basePanel.output(Localization.lang("Looking for full text document..."));
} else {
Expand All @@ -68,21 +60,24 @@ private Map<Optional<URL>, 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<Optional<URL>, BibEntry> findFullTexts() {
Map<Optional<URL>, 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<Optional<URL>, BibEntry> downloads) {
List<Optional<URL>> finishedTasks = new ArrayList<>();
for (Map.Entry<Optional<URL>, BibEntry> download : downloads.entrySet()) {
BibEntry entry = download.getValue();
Optional<URL> result = download.getKey();
Expand All @@ -94,20 +89,15 @@ private void downloadFullTexts(Map<Optional<URL>, 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<URL> result : finishedTasks) {
downloads.remove(result);
}
}

Expand All @@ -119,7 +109,6 @@ private void downloadFullTexts(Map<Optional<URL>, BibEntry> downloads) {
* @param entry the entry "value"
*/
private void addLinkedFileFromURL(URL url, BibEntry entry) {

LinkedFile newLinkedFile = new LinkedFile(url, "");

if (!entry.getFiles().contains(newLinkedFile)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/jabref/gui/importer/ImportAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -95,11 +96,11 @@ private List<ImportFormatReader.UnknownFormatImport> doImport(List<Path> files)
try {
if (!importer.isPresent()) {
// Unknown format:
frame.getDialogService().notify(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.getDialogService().notify(Localization.lang("Importing in %0 format", importer.get().getName()) + "...");
DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().notify(Localization.lang("Importing in %0 format", importer.get().getName()) + "..."));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also a workaround here and i would suggest to remove the notifications

// Specific importer:
ParserResult pr = importer.get().importDatabase(filename, Globals.prefs.getDefaultEncoding());
imports.add(new ImportFormatReader.UnknownFormatImport(importer.get().getName(), pr));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 + "'");})
Copy link
Member

Choose a reason for hiding this comment

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

singleLineLambdas usually don't need curly braces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it :)

.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();
}

}
3 changes: 1 addition & 2 deletions src/main/java/org/jabref/gui/journals/AbbreviateAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,14 @@ 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);

}

private String abbreviate() {
panel.output(Localization.lang("Abbreviating..."));

List<BibEntry> entries = panel.getSelectedEntries();
UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator(
Globals.journalAbbreviationLoader.getRepository(Globals.prefs.getJournalAbbreviationPreferences()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<BibEntry> entries = panel.getSelectedEntries(); // never null

UndoableUnabbreviator undoableAbbreviator = new UndoableUnabbreviator(Globals.journalAbbreviationLoader
Expand Down