From 7269d2771d0692d3b3f1cc086bac2f37d7e47fa4 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Fri, 28 Dec 2018 20:52:37 +0100 Subject: [PATCH 1/3] Converts integrity check dialog to JavaFX Moreover: - Show entry by reference and not by id. Fixes #2181. - Fixes a few issues that occurred when opening the entry editor by code from the integrity dialog - Reuse gridpane in entry editor (should have a slightly superior performance) - Improve display of progress dialog - Invoke copy files task using central task executor --- src/main/java/org/jabref/gui/BasePanel.java | 10 +- .../java/org/jabref/gui/DialogService.java | 4 +- .../java/org/jabref/gui/FXDialogService.java | 9 +- src/main/java/org/jabref/gui/JabRefFrame.java | 2 +- .../jabref/gui/actions/CopyFilesAction.java | 17 +- .../gui/actions/IntegrityCheckAction.java | 202 ------------------ .../gui/entryeditor/FieldsEditorTab.java | 45 ++-- .../gui/integrity/IntegrityCheckAction.java | 79 +++++++ .../gui/integrity/IntegrityCheckDialog.fxml | 26 +++ .../gui/integrity/IntegrityCheckDialog.java | 65 ++++++ .../IntegrityCheckDialogViewModel.java | 22 ++ .../gui/util/CurrentThreadTaskExecutor.java | 11 +- .../jabref/gui/util/DefaultTaskExecutor.java | 10 +- .../org/jabref/gui/util/TaskExecutor.java | 11 +- .../logic/integrity/IntegrityCheck.java | 2 +- 15 files changed, 271 insertions(+), 244 deletions(-) delete mode 100644 src/main/java/org/jabref/gui/actions/IntegrityCheckAction.java create mode 100644 src/main/java/org/jabref/gui/integrity/IntegrityCheckAction.java create mode 100644 src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.fxml create mode 100644 src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.java create mode 100644 src/main/java/org/jabref/gui/integrity/IntegrityCheckDialogViewModel.java diff --git a/src/main/java/org/jabref/gui/BasePanel.java b/src/main/java/org/jabref/gui/BasePanel.java index 9155e2fc3e9..a2e7e5734b6 100644 --- a/src/main/java/org/jabref/gui/BasePanel.java +++ b/src/main/java/org/jabref/gui/BasePanel.java @@ -19,6 +19,7 @@ import javax.swing.undo.CannotRedoException; import javax.swing.undo.CannotUndoException; +import javafx.application.Platform; import javafx.beans.binding.Bindings; import javafx.geometry.Orientation; import javafx.scene.Node; @@ -693,11 +694,12 @@ public void insertEntry(final BibEntry bibEntry) { } } - public void editEntryByIdAndFocusField(final String entryId, final String fieldName) { - bibDatabaseContext.getDatabase().getEntryById(entryId).ifPresent(entry -> { - clearAndSelect(entry); - showAndEdit(entry); + public void editEntryAndFocusField(BibEntry entry, String fieldName) { + showAndEdit(entry); + Platform.runLater(() -> { + // Focus field and entry in main table (async to give entry editor time to load) entryEditor.setFocusToField(fieldName); + clearAndSelect(entry); }); } diff --git a/src/main/java/org/jabref/gui/DialogService.java b/src/main/java/org/jabref/gui/DialogService.java index 304efe82289..f776d192671 100644 --- a/src/main/java/org/jabref/gui/DialogService.java +++ b/src/main/java/org/jabref/gui/DialogService.java @@ -183,9 +183,11 @@ Optional showCustomButtonDialogAndWait(Alert.AlertType type, String /** * Constructs and shows a canceable {@link ProgressDialog}. Clicking cancel will cancel the underlying service and close the dialog * + * @param title title of the dialog + * @param content message to show above the progress bar * @param task The {@link Task} which executes the work and for which to show the dialog */ - void showCanceableProgressDialogAndWait(Task task); + void showProgressDialogAndWait(String title, String content, Task task); /** * Notify the user in an non-blocking way (i.e., in form of toast in a snackbar). diff --git a/src/main/java/org/jabref/gui/FXDialogService.java b/src/main/java/org/jabref/gui/FXDialogService.java index 0b25b524cd6..60593d1edf9 100644 --- a/src/main/java/org/jabref/gui/FXDialogService.java +++ b/src/main/java/org/jabref/gui/FXDialogService.java @@ -24,9 +24,11 @@ import javafx.scene.layout.Region; import javafx.stage.DirectoryChooser; import javafx.stage.FileChooser; +import javafx.stage.Stage; import javafx.stage.Window; 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.logic.l10n.Localization; @@ -226,8 +228,13 @@ public Optional showCustomDialogAndWait(Dialog dialog) { } @Override - public void showCanceableProgressDialogAndWait(Task task) { + public void showProgressDialogAndWait(String title, String content, Task task) { ProgressDialog progressDialog = new ProgressDialog(task); + progressDialog.setHeaderText(null); + progressDialog.setTitle(title); + progressDialog.setContentText(content); + progressDialog.setGraphic(null); + ((Stage) progressDialog.getDialogPane().getScene().getWindow()).getIcons().add(IconTheme.getJabRefImageFX()); progressDialog.setOnCloseRequest(evt -> task.cancel()); DialogPane dialogPane = progressDialog.getDialogPane(); dialogPane.getButtonTypes().add(ButtonType.CANCEL); diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index bc3a4e8a722..86c9f7ec54a 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -62,7 +62,6 @@ import org.jabref.gui.actions.DatabasePropertiesAction; import org.jabref.gui.actions.EditExternalFileTypesAction; import org.jabref.gui.actions.ErrorConsoleAction; -import org.jabref.gui.actions.IntegrityCheckAction; import org.jabref.gui.actions.LookupIdentifierAction; import org.jabref.gui.actions.ManageCustomExportsAction; import org.jabref.gui.actions.ManageCustomImportsAction; @@ -94,6 +93,7 @@ import org.jabref.gui.importer.ImportCommand; import org.jabref.gui.importer.ImportInspectionDialog; import org.jabref.gui.importer.actions.OpenDatabaseAction; +import org.jabref.gui.integrity.IntegrityCheckAction; import org.jabref.gui.keyboard.KeyBinding; import org.jabref.gui.menus.FileHistoryMenu; import org.jabref.gui.mergeentries.MergeEntriesAction; diff --git a/src/main/java/org/jabref/gui/actions/CopyFilesAction.java b/src/main/java/org/jabref/gui/actions/CopyFilesAction.java index 5efa6415506..d45a78eb803 100644 --- a/src/main/java/org/jabref/gui/actions/CopyFilesAction.java +++ b/src/main/java/org/jabref/gui/actions/CopyFilesAction.java @@ -32,16 +32,6 @@ public CopyFilesAction(JabRefFrame frame) { this.dialogService = frame.getDialogService(); } - private void startServiceAndshowProgessDialog(Task> exportService) { - - dialogService.showCanceableProgressDialogAndWait(exportService); - - exportService.run(); - exportService.setOnSucceeded((e) -> { - showDialog(exportService.getValue()); - }); - } - private void showDialog(List data) { if (data.isEmpty()) { dialogService.showInformationDialogAndWait(Localization.lang("Copy linked files to folder..."), Localization.lang("No linked files found for export.")); @@ -64,7 +54,12 @@ public void execute() { databaseContext = frame.getCurrentBasePanel().getBibDatabaseContext(); Task> exportTask = new CopyFilesTask(databaseContext, entries, path); - startServiceAndshowProgessDialog(exportTask); + dialogService.showProgressDialogAndWait( + Localization.lang("Copy linked files to folder..."), + Localization.lang("Copy linked files to folder..."), + exportTask); + Globals.TASK_EXECUTOR.execute(exportTask); + exportTask.setOnSucceeded((e) -> showDialog(exportTask.getValue())); }); } diff --git a/src/main/java/org/jabref/gui/actions/IntegrityCheckAction.java b/src/main/java/org/jabref/gui/actions/IntegrityCheckAction.java deleted file mode 100644 index ad07a4dde80..00000000000 --- a/src/main/java/org/jabref/gui/actions/IntegrityCheckAction.java +++ /dev/null @@ -1,202 +0,0 @@ -package org.jabref.gui.actions; - -import java.awt.Component; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import javax.swing.JButton; -import javax.swing.JCheckBoxMenuItem; -import javax.swing.JDialog; -import javax.swing.JFrame; -import javax.swing.JOptionPane; -import javax.swing.JPopupMenu; -import javax.swing.JProgressBar; -import javax.swing.JScrollPane; -import javax.swing.JTable; -import javax.swing.ListSelectionModel; -import javax.swing.RowFilter; -import javax.swing.SwingWorker; -import javax.swing.table.AbstractTableModel; -import javax.swing.table.TableColumnModel; -import javax.swing.table.TableModel; -import javax.swing.table.TableRowSorter; - -import org.jabref.Globals; -import org.jabref.gui.JabRefFrame; -import org.jabref.logic.integrity.IntegrityCheck; -import org.jabref.logic.integrity.IntegrityMessage; -import org.jabref.logic.l10n.Localization; -import org.jabref.preferences.JabRefPreferences; - -import com.jgoodies.forms.builder.FormBuilder; -import com.jgoodies.forms.layout.FormLayout; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class IntegrityCheckAction extends SimpleCommand { - - private static final Logger LOGGER = LoggerFactory.getLogger(IntegrityCheckAction.class); - private static final String ELLIPSES = "..."; - - private final JabRefFrame frame; - - public IntegrityCheckAction(JabRefFrame frame) { - this.frame = frame; - } - - @Override - public void execute() { - IntegrityCheck check = new IntegrityCheck(frame.getCurrentBasePanel().getBibDatabaseContext(), - Globals.prefs.getFilePreferences(), - Globals.prefs.getBibtexKeyPatternPreferences(), - Globals.journalAbbreviationLoader.getRepository(Globals.prefs.getJournalAbbreviationPreferences()), - Globals.prefs.getBoolean(JabRefPreferences.ENFORCE_LEGAL_BIBTEX_KEY)); - - final JDialog integrityDialog = new JDialog((JFrame) null, true); - integrityDialog.setUndecorated(true); - JProgressBar integrityProgressBar = new JProgressBar(); - integrityProgressBar.setIndeterminate(true); - integrityProgressBar.setStringPainted(true); - integrityProgressBar.setString(Localization.lang("Checking integrity...")); - integrityDialog.add(integrityProgressBar); - integrityDialog.pack(); - SwingWorker, Void> worker = new SwingWorker, Void>() { - @Override - protected List doInBackground() { - List messages = check.checkBibtexDatabase(); - return messages; - } - - @Override - protected void done() { - integrityDialog.dispose(); - } - }; - worker.execute(); - integrityDialog.setVisible(true); - List messages = null; - try { - messages = worker.get(); - } catch (Exception ex) { - LOGGER.error("Integrity check failed.", ex); - } - - if (messages.isEmpty()) { - JOptionPane.showMessageDialog(null, Localization.lang("No problems found.")); - } else { - Map showMessage = new HashMap<>(); - // prepare data model - Object[][] model = new Object[messages.size()][4]; - int i = 0; - for (IntegrityMessage message : messages) { - model[i][0] = message.getEntry().getId(); - model[i][1] = message.getEntry().getCiteKeyOptional().orElse(""); - model[i][2] = message.getFieldName(); - model[i][3] = message.getMessage(); - showMessage.put(message.getMessage(), true); - i++; - } - - // construct view - JTable table = new JTable(model, - new Object[] {"ID", Localization.lang("BibTeX key"), Localization.lang("Field"), - Localization.lang("Message")}); - - // hide IDs - TableColumnModel columnModel = table.getColumnModel(); - columnModel.removeColumn(columnModel.getColumn(0)); - - RowFilter filter = new RowFilter() { - - @Override - public boolean include(Entry entry) { - return showMessage.get(entry.getStringValue(3)); - } - }; - - TableRowSorter sorter = new TableRowSorter<>(table.getModel()); - sorter.setRowFilter(filter); - table.setRowSorter(sorter); - table.setSelectionMode(ListSelectionModel.SINGLE_SELECTION); - table.setDefaultEditor(Object.class, null); - ListSelectionModel selectionModel = table.getSelectionModel(); - - selectionModel.addListSelectionListener(event -> { - if (!event.getValueIsAdjusting()) { - try { - String entryId = (String) model[table.convertRowIndexToModel(table.getSelectedRow())][0]; - String fieldName = (String) model[table.convertRowIndexToModel(table.getSelectedRow())][2]; - frame.getCurrentBasePanel().editEntryByIdAndFocusField(entryId, fieldName); - } catch (ArrayIndexOutOfBoundsException exception) { - // Ignore -- most likely caused by filtering out the earlier selected row - } - } - }); - - // BibTeX key - table.getColumnModel().getColumn(0).setPreferredWidth(100); - // field name - table.getColumnModel().getColumn(1).setPreferredWidth(60); - // message - table.getColumnModel().getColumn(2).setPreferredWidth(400); - table.setAutoResizeMode(JTable.AUTO_RESIZE_LAST_COLUMN); - JScrollPane scrollPane = new JScrollPane(table); - String title = Localization.lang("%0 problem(s) found", String.valueOf(messages.size())); - JDialog dialog = new JDialog((JFrame) null, title, false); - - JPopupMenu menu = new JPopupMenu(); - for (String messageString : showMessage.keySet()) { - JCheckBoxMenuItem menuItem = new JCheckBoxMenuItem(messageString, true); - menuItem.addActionListener(event -> { - showMessage.put(messageString, menuItem.isSelected()); - ((AbstractTableModel) table.getModel()).fireTableDataChanged(); - }); - menu.add(menuItem); - } - - JButton menuButton = new JButton(Localization.lang("Filter")); - menuButton.addActionListener(entry -> menu.show(menuButton, 0, menuButton.getHeight())); - FormBuilder builder = FormBuilder.create() - .layout(new FormLayout("fill:pref:grow", "fill:pref:grow, 2dlu, pref")); - - JButton filterNoneButton = new JButton(Localization.lang("Filter None")); - filterNoneButton.addActionListener(event -> { - for (Component component : menu.getComponents()) { - if (component instanceof JCheckBoxMenuItem) { - JCheckBoxMenuItem checkBox = (JCheckBoxMenuItem) component; - if (checkBox.isSelected()) { - checkBox.setSelected(false); - showMessage.put(checkBox.getText(), checkBox.isSelected()); - } - } - ((AbstractTableModel) table.getModel()).fireTableDataChanged(); - } - }); - - JButton filterAllButton = new JButton(Localization.lang("Filter All")); - filterAllButton.addActionListener(event -> { - for (Component component : menu.getComponents()) { - if (component instanceof JCheckBoxMenuItem) { - JCheckBoxMenuItem checkBox = (JCheckBoxMenuItem) component; - if (!checkBox.isSelected()) { - checkBox.setSelected(true); - showMessage.put(checkBox.getText(), checkBox.isSelected()); - } - } - ((AbstractTableModel) table.getModel()).fireTableDataChanged(); - } - }); - - builder.add(filterNoneButton).xy(1, 3, "left, b"); - builder.add(filterAllButton).xy(1, 3, "right, b"); - builder.add(scrollPane).xy(1, 1); - builder.add(menuButton).xy(1, 3, "c, b"); - dialog.add(builder.getPanel()); - dialog.setSize(600, 600); - - // show view - dialog.setVisible(true); - } - } -} diff --git a/src/main/java/org/jabref/gui/entryeditor/FieldsEditorTab.java b/src/main/java/org/jabref/gui/entryeditor/FieldsEditorTab.java index cdaebfda242..bbd23b29d97 100644 --- a/src/main/java/org/jabref/gui/entryeditor/FieldsEditorTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/FieldsEditorTab.java @@ -50,8 +50,9 @@ abstract class FieldsEditorTab extends EntryEditorTab { private FieldEditorFX activeField; private final BibDatabaseContext databaseContext; private UndoManager undoManager; - private Collection fields; + private Collection fields = new ArrayList<>(); private final DialogService dialogService; + private GridPane gridPane; public FieldsEditorTab(boolean compressed, BibDatabaseContext databaseContext, SuggestionProviders suggestionProviders, UndoManager undoManager, DialogService dialogService) { this.isCompressed = compressed; @@ -69,14 +70,17 @@ private static void addColumn(GridPane gridPane, int columnIndex, Stream gridPane.addColumn(columnIndex, nodes.toArray(Node[]::new)); } - private Region setupPanel(BibEntry entry, boolean compressed, SuggestionProviders suggestionProviders, UndoManager undoManager) { - // The preferences might be not initialized in tests -> return empty node + private void setupPanel(BibEntry entry, boolean compressed, SuggestionProviders suggestionProviders, UndoManager undoManager) { + // The preferences might be not initialized in tests -> return immediately // TODO: Replace this ugly workaround by proper injection propagation if (Globals.prefs == null) { - return new Region(); + return; } editors.clear(); + gridPane.getChildren().clear(); + gridPane.getColumnConstraints().clear(); + gridPane.getRowConstraints().clear(); EntryType entryType = EntryTypes.getTypeOrDefault(entry.getType(), databaseContext.getMode()); fields = determineFieldsToShow(entry, entryType); @@ -98,9 +102,6 @@ private Region setupPanel(BibEntry entry, boolean compressed, SuggestionProvider labels.add(new FieldNameLabel(fieldName)); } - GridPane gridPane = new GridPane(); - gridPane.getStyleClass().add("editorPane"); - ColumnConstraints columnExpand = new ColumnConstraints(); columnExpand.setHgrow(Priority.ALWAYS); @@ -127,15 +128,6 @@ private Region setupPanel(BibEntry entry, boolean compressed, SuggestionProvider setRegularRowLayout(gridPane); } - - // Warp everything in a scroll-pane - ScrollPane scrollPane = new ScrollPane(); - scrollPane.setHbarPolicy(ScrollPane.ScrollBarPolicy.NEVER); - scrollPane.setVbarPolicy(ScrollPane.ScrollBarPolicy.AS_NEEDED); - scrollPane.setContent(gridPane); - scrollPane.setFitToWidth(true); - scrollPane.setFitToHeight(true); - return scrollPane; } private void setRegularRowLayout(GridPane gridPane) { @@ -222,8 +214,8 @@ protected void bindToEntry(BibEntry entry) { .map(Map.Entry::getKey) .findFirst(); - Region panel = setupPanel(entry, isCompressed, suggestionProviders, undoManager); - setContent(panel); + initPanel(); + setupPanel(entry, isCompressed, suggestionProviders, undoManager); Platform.runLater(() -> { // Restore focus to field (run this async so that editor is already initialized correctly) @@ -236,4 +228,21 @@ protected void bindToEntry(BibEntry entry) { public Collection getShownFields() { return fields; } + + private void initPanel() { + if (gridPane == null) { + gridPane = new GridPane(); + gridPane.getStyleClass().add("editorPane"); + + // Warp everything in a scroll-pane + ScrollPane scrollPane = new ScrollPane(); + scrollPane.setHbarPolicy(ScrollPane.ScrollBarPolicy.NEVER); + scrollPane.setVbarPolicy(ScrollPane.ScrollBarPolicy.AS_NEEDED); + scrollPane.setContent(gridPane); + scrollPane.setFitToWidth(true); + scrollPane.setFitToHeight(true); + + setContent(scrollPane); + } + } } diff --git a/src/main/java/org/jabref/gui/integrity/IntegrityCheckAction.java b/src/main/java/org/jabref/gui/integrity/IntegrityCheckAction.java new file mode 100644 index 00000000000..3bc53cb180e --- /dev/null +++ b/src/main/java/org/jabref/gui/integrity/IntegrityCheckAction.java @@ -0,0 +1,79 @@ +package org.jabref.gui.integrity; + +import java.util.ArrayList; +import java.util.List; + +import javafx.collections.ObservableList; +import javafx.concurrent.Task; + +import org.jabref.Globals; +import org.jabref.gui.Dialog; +import org.jabref.gui.DialogService; +import org.jabref.gui.JabRefFrame; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.util.TaskExecutor; +import org.jabref.logic.integrity.IntegrityCheck; +import org.jabref.logic.integrity.IntegrityMessage; +import org.jabref.logic.l10n.Localization; +import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.BibEntry; +import org.jabref.preferences.JabRefPreferences; + +public class IntegrityCheckAction extends SimpleCommand { + + private final JabRefFrame frame; + private final TaskExecutor taskExecutor; + private final DialogService dialogService; + + public IntegrityCheckAction(JabRefFrame frame) { + this.frame = frame; + this.taskExecutor = Globals.TASK_EXECUTOR; + this.dialogService = frame.getDialogService(); + } + + @Override + public void execute() { + BibDatabaseContext databaseContext = frame.getCurrentBasePanel().getBibDatabaseContext(); + IntegrityCheck check = new IntegrityCheck(databaseContext, + Globals.prefs.getFilePreferences(), + Globals.prefs.getBibtexKeyPatternPreferences(), + Globals.journalAbbreviationLoader.getRepository(Globals.prefs.getJournalAbbreviationPreferences()), + Globals.prefs.getBoolean(JabRefPreferences.ENFORCE_LEGAL_BIBTEX_KEY)); + + Task> task = new Task>() { + @Override + protected List call() { + List result = new ArrayList<>(); + + ObservableList entries = databaseContext.getDatabase().getEntries(); + for (int i = 0; i < entries.size(); i++) { + if (isCancelled()) { + break; + } + + BibEntry entry = entries.get(i); + result.addAll(check.checkBibtexEntry(entry)); + updateProgress(i, entries.size()); + } + + return result; + } + }; + task.setOnSucceeded(value -> { + List messages = task.getValue(); + if (messages.isEmpty()) { + dialogService.notify(Localization.lang("No problems found.")); + } else { + Dialog dialog = new IntegrityCheckDialog(messages, frame.getCurrentBasePanel()); + dialogService.showCustomDialogAndWait(dialog); + } + }); + task.setOnFailed(event -> dialogService.showErrorDialogAndWait("Integrity check failed.")); + + dialogService.showProgressDialogAndWait( + Localization.lang("Checking integrity..."), + Localization.lang("Checking integrity..."), + task); + taskExecutor.execute(task); + } +} diff --git a/src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.fxml b/src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.fxml new file mode 100644 index 00000000000..e3eb9580ce0 --- /dev/null +++ b/src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.fxml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.java b/src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.java new file mode 100644 index 00000000000..cd6277bf322 --- /dev/null +++ b/src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.java @@ -0,0 +1,65 @@ +package org.jabref.gui.integrity; + +import java.util.List; + +import javafx.beans.property.ReadOnlyStringWrapper; +import javafx.collections.ListChangeListener; +import javafx.fxml.FXML; +import javafx.scene.control.TableColumn; +import javafx.scene.control.TableView; +import javafx.stage.Modality; + +import org.jabref.gui.BasePanel; +import org.jabref.gui.util.BaseDialog; +import org.jabref.logic.integrity.IntegrityMessage; +import org.jabref.logic.l10n.Localization; + +import com.airhacks.afterburner.views.ViewLoader; +import org.controlsfx.control.table.TableFilter; + +public class IntegrityCheckDialog extends BaseDialog { + + private final List messages; + private final BasePanel basePanel; + @FXML private TableView messagesTable; + @FXML private TableColumn keyColumn; + @FXML private TableColumn fieldColumn; + @FXML private TableColumn messageColumn; + private IntegrityCheckDialogViewModel viewModel; + + public IntegrityCheckDialog(List messages, BasePanel basePanel) { + this.messages = messages; + this.basePanel = basePanel; + this.setTitle(Localization.lang("Check integrity")); + this.initModality(Modality.NONE); + + ViewLoader.view(this) + .load() + .setAsDialogPane(this); + } + + private void onSelectionChanged(ListChangeListener.Change change) { + if (change.next()) { + change.getAddedSubList().stream().findFirst().ifPresent(message -> + basePanel.editEntryAndFocusField(message.getEntry(), message.getFieldName())); + } + } + + public IntegrityCheckDialogViewModel getViewModel() { + return viewModel; + } + + @FXML + private void initialize() { + viewModel = new IntegrityCheckDialogViewModel(messages); + + messagesTable.getSelectionModel().getSelectedItems().addListener(this::onSelectionChanged); + messagesTable.setItems(viewModel.getMessages()); + keyColumn.setCellValueFactory(row -> new ReadOnlyStringWrapper(row.getValue().getEntry().getCiteKeyOptional().orElse(""))); + fieldColumn.setCellValueFactory(row -> new ReadOnlyStringWrapper(row.getValue().getFieldName())); + messageColumn.setCellValueFactory(row -> new ReadOnlyStringWrapper(row.getValue().getMessage())); + + TableFilter.forTableView(messagesTable) + .apply(); + } +} diff --git a/src/main/java/org/jabref/gui/integrity/IntegrityCheckDialogViewModel.java b/src/main/java/org/jabref/gui/integrity/IntegrityCheckDialogViewModel.java new file mode 100644 index 00000000000..1313417642e --- /dev/null +++ b/src/main/java/org/jabref/gui/integrity/IntegrityCheckDialogViewModel.java @@ -0,0 +1,22 @@ +package org.jabref.gui.integrity; + +import java.util.List; + +import javafx.collections.FXCollections; +import javafx.collections.ObservableList; + +import org.jabref.gui.AbstractViewModel; +import org.jabref.logic.integrity.IntegrityMessage; + +public class IntegrityCheckDialogViewModel extends AbstractViewModel { + + private final ObservableList messages; + + public IntegrityCheckDialogViewModel(List messages) { + this.messages = FXCollections.observableArrayList(messages); + } + + public ObservableList getMessages() { + return messages; + } +} diff --git a/src/main/java/org/jabref/gui/util/CurrentThreadTaskExecutor.java b/src/main/java/org/jabref/gui/util/CurrentThreadTaskExecutor.java index 31da770b4e6..a18bf6fc5aa 100644 --- a/src/main/java/org/jabref/gui/util/CurrentThreadTaskExecutor.java +++ b/src/main/java/org/jabref/gui/util/CurrentThreadTaskExecutor.java @@ -6,6 +6,8 @@ import java.util.concurrent.TimeUnit; import java.util.function.Consumer; +import javafx.concurrent.Task; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -23,7 +25,7 @@ public class CurrentThreadTaskExecutor implements TaskExecutor { * javafx.concurrent.Task.TaskCallable#call()}, but adapted to run sequentially. */ @Override - public Future execute(BackgroundTask task) { + public Future execute(BackgroundTask task) { Runnable onRunning = task.getOnRunning(); if (onRunning != null) { onRunning.run(); @@ -42,10 +44,15 @@ public Future execute(BackgroundTask task) { } else { LOGGER.error("Unhandled exception", exception); } - return new FailedFuture(exception); + return new FailedFuture<>(exception); } } + @Override + public Future execute(Task task) { + return task; + } + @Override public void shutdown() { // Nothing to do here diff --git a/src/main/java/org/jabref/gui/util/DefaultTaskExecutor.java b/src/main/java/org/jabref/gui/util/DefaultTaskExecutor.java index fe7f2f64611..b1cde500093 100644 --- a/src/main/java/org/jabref/gui/util/DefaultTaskExecutor.java +++ b/src/main/java/org/jabref/gui/util/DefaultTaskExecutor.java @@ -89,8 +89,14 @@ public static void runInJavaFXThread(Runnable runnable) { } @Override - public Future execute(BackgroundTask task) { - return EXECUTOR.submit(getJavaFXTask(task)); + public Future execute(BackgroundTask task) { + return execute(getJavaFXTask(task)); + } + + @Override + public Future execute(Task task) { + EXECUTOR.submit(task); + return task; } @Override diff --git a/src/main/java/org/jabref/gui/util/TaskExecutor.java b/src/main/java/org/jabref/gui/util/TaskExecutor.java index 5fba6535f16..0d5f2f8c8f3 100644 --- a/src/main/java/org/jabref/gui/util/TaskExecutor.java +++ b/src/main/java/org/jabref/gui/util/TaskExecutor.java @@ -18,7 +18,16 @@ public interface TaskExecutor { * @param type of return value of the task * @param task the task to run */ - Future execute(BackgroundTask task); + Future execute(BackgroundTask task); + + /** + * Runs the given task and returns a Future representing that task. Usually, you want to use the other method {@link + * #execute(BackgroundTask)}. + * + * @param type of return value of the task + * @param task the task to run + */ + Future execute(Task task); /** * Shutdown the task executor. diff --git a/src/main/java/org/jabref/logic/integrity/IntegrityCheck.java b/src/main/java/org/jabref/logic/integrity/IntegrityCheck.java index 7932c842ae1..9e534756bce 100644 --- a/src/main/java/org/jabref/logic/integrity/IntegrityCheck.java +++ b/src/main/java/org/jabref/logic/integrity/IntegrityCheck.java @@ -41,7 +41,7 @@ public List checkBibtexDatabase() { return result; } - private List checkBibtexEntry(BibEntry entry) { + public List checkBibtexEntry(BibEntry entry) { List result = new ArrayList<>(); if (entry == null) { From e4ba2779deeb7dd8d9098e57837c06c5af48f2ca Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 4 Jan 2019 17:59:59 +0100 Subject: [PATCH 2/3] fix l10n fix aborting of copy files task and showing of integrity check dialog --- .../jabref/gui/copyfiles/CopyFilesTask.java | 23 +++++++++++++++---- .../gui/integrity/IntegrityCheckAction.java | 2 +- src/main/resources/l10n/JabRef_en.properties | 4 ---- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/jabref/gui/copyfiles/CopyFilesTask.java b/src/main/java/org/jabref/gui/copyfiles/CopyFilesTask.java index aec95051171..ee562c53401 100644 --- a/src/main/java/org/jabref/gui/copyfiles/CopyFilesTask.java +++ b/src/main/java/org/jabref/gui/copyfiles/CopyFilesTask.java @@ -67,9 +67,18 @@ protected List call() throws InterruptedException, for (int i = 0; i < entries.size(); i++) { + if (isCancelled()) { + break; + } + List files = entries.get(i).getFiles(); for (int j = 0; j < files.size(); j++) { + + if (isCancelled()) { + break; + } + updateMessage(Localization.lang("Copying file %0 of entry %1", Integer.toString(j + 1), Integer.toString(i + 1))); LinkedFile fileName = files.get(j); @@ -78,14 +87,18 @@ protected List call() throws InterruptedException, newPath = OptionalUtil.combine(Optional.of(exportPath), fileToExport, resolvePathFilename); - newPath.ifPresent(newFile -> { + if (newPath.isPresent()) { + + Path newFile = newPath.get(); boolean success = FileUtil.copyFile(fileToExport.get(), newFile, false); updateProgress(totalFilesCounter++, totalFilesCount); try { Thread.sleep(300); } catch (InterruptedException e) { - // TODO Auto-generated catch block - e.printStackTrace(); + if (isCancelled()) { + updateMessage("Cancelled"); + break; + } } if (success) { updateMessage(localizedSucessMessage); @@ -99,7 +112,9 @@ protected List call() throws InterruptedException, writeLogMessage(newFile, bw, localizedErrorMessage); addResultToList(newFile, success, localizedErrorMessage); } - }); + + } + } } updateMessage(Localization.lang("Finished copying")); diff --git a/src/main/java/org/jabref/gui/integrity/IntegrityCheckAction.java b/src/main/java/org/jabref/gui/integrity/IntegrityCheckAction.java index 3bc53cb180e..0bf4c182db8 100644 --- a/src/main/java/org/jabref/gui/integrity/IntegrityCheckAction.java +++ b/src/main/java/org/jabref/gui/integrity/IntegrityCheckAction.java @@ -65,7 +65,7 @@ protected List call() { dialogService.notify(Localization.lang("No problems found.")); } else { Dialog dialog = new IntegrityCheckDialog(messages, frame.getCurrentBasePanel()); - dialogService.showCustomDialogAndWait(dialog); + dialog.showAndWait(); } }); task.setOnFailed(event -> dialogService.showErrorDialogAndWait("Integrity check failed.")); diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 1af3cc0c045..4ef9bf2dc56 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -433,10 +433,6 @@ Files\ opened=Files opened Filter=Filter -Filter\ All=Filter All - -Filter\ None=Filter None - Finished\ automatically\ setting\ external\ links.=Finished automatically setting external links. Finished\ writing\ XMP\ for\ %0\ file\ (%1\ skipped,\ %2\ errors).=Finished writing XMP for %0 file (%1 skipped, %2 errors). From 00b6f13f3d71096976d522a2e81102915cbc34a6 Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Fri, 4 Jan 2019 18:36:40 +0100 Subject: [PATCH 3/3] fix l10n --- src/main/resources/l10n/JabRef_en.properties | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 9454f8812fb..87b3ffe2dca 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -1601,7 +1601,6 @@ Keep\ right=Keep right Old\ entry=Old entry From\ import=From import No\ problems\ found.=No problems found. -%0\ problem(s)\ found=%0 problem(s) found Save\ changes=Save changes Discard\ changes=Discard changes Library\ '%0'\ has\ changed.=Library '%0' has changed.