Merge remote-tracking branch 'upstream/master' into convertSearchWorker

* upstream/master:
  Fix for the issue #4912 (#4916)
  Refactorings, move functionality of CustomEntryTypesManager to Preferences Use ObservableList instead of LIstProperty
  Toggle enable status of menu items (#4872)
  Fixes throwing an exception when 'id' field is present in bib file (#4918)
  Quick fix for error when opening preferences (#4917)
  Revert "Create new layout for preferences regarding columns"
  Create new layout for preferences regarding columns
  Rearrange the padding of the "Append library" dialog (#4914)
  Make Group dialog resizable (#4910)
  Adjust save exception to inlcude orgininal stack traces
  remove dialogService parameter
  remove dialog service argument
  rework dialog, create fxml etc fix l10n Remove obsolete code
  rework threading stuff simplify code
  Rework PosteOpenActions to javafx (custom entry type import)
  Rework PosteOpenActions to javafx (custom entry type import)

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

@@ -30,20 +30,20 @@
 import org.jabref.JabRefExecutorService;
 import org.jabref.gui.actions.Actions;
 import org.jabref.gui.actions.BaseAction;
-import org.jabref.gui.actions.CleanupAction;
-import org.jabref.gui.actions.CopyBibTeXKeyAndLinkAction;
-import org.jabref.gui.actions.GenerateBibtexKeyAction;
-import org.jabref.gui.actions.WriteXMPAction;
 import org.jabref.gui.autocompleter.AutoCompletePreferences;
 import org.jabref.gui.autocompleter.AutoCompleteUpdater;
 import org.jabref.gui.autocompleter.PersonNameSuggestionProvider;
 import org.jabref.gui.autocompleter.SuggestionProviders;
+import org.jabref.gui.bibtexkeypattern.GenerateBibtexKeyAction;
+import org.jabref.gui.cleanup.CleanupAction;
 import org.jabref.gui.collab.DatabaseChangeMonitor;
 import org.jabref.gui.collab.DatabaseChangePane;
 import org.jabref.gui.desktop.JabRefDesktop;
+import org.jabref.gui.edit.CopyBibTeXKeyAndLinkAction;
 import org.jabref.gui.edit.ReplaceStringAction;
 import org.jabref.gui.entryeditor.EntryEditor;
 import org.jabref.gui.exporter.SaveDatabaseAction;
+import org.jabref.gui.exporter.WriteXMPAction;
 import org.jabref.gui.externalfiles.FindFullTextAction;
 import org.jabref.gui.externalfiletype.ExternalFileMenuItem;
 import org.jabref.gui.externalfiletype.ExternalFileType;
@@ -294,7 +294,7 @@ private void setupActions() {
         // The action for cleaning up entry.
         actions.put(Actions.CLEANUP, cleanUpAction);
 
-        actions.put(Actions.MERGE_ENTRIES, () -> new MergeEntriesAction(frame).execute());
+        actions.put(Actions.MERGE_ENTRIES, () -> new MergeEntriesAction(frame, Globals.stateManager).execute());
 
         // The action for copying the selected entry's key.
         actions.put(Actions.COPY_KEY, this::copyKey);
@@ -432,7 +432,7 @@ private void delete(boolean cut, List<BibEntry> entries) {
             compound = new NamedCompound((entries.size() > 1 ? Localization.lang("delete entries") : Localization.lang("delete entry")));
         }
         for (BibEntry entry : entries) {
-            compound.addEdit(new UndoableRemoveEntry(bibDatabaseContext.getDatabase(), entry, BasePanel.this));
+            compound.addEdit(new UndoableRemoveEntry(bibDatabaseContext.getDatabase(), entry));
             bibDatabaseContext.getDatabase().removeEntry(entry);
             ensureNotShowingBottomPanel(entry);
         }

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

@@ -14,6 +14,7 @@
 import javafx.concurrent.Task;
 import javafx.concurrent.Worker;
 
+import org.jabref.gui.duplicationFinder.DuplicateResolverDialog;
 import org.jabref.logic.bibtex.DuplicateCheck;
 import org.jabref.logic.bibtexkeypattern.BibtexKeyGenerator;
 import org.jabref.logic.importer.FetcherException;

src/main/java/org/jabref/gui/SaveOrderConfigDisplay.fxml

@@ -8,7 +8,6 @@
 <?import javafx.scene.layout.ColumnConstraints?>
 <?import javafx.scene.layout.GridPane?>
 <?import javafx.scene.layout.RowConstraints?>
-
 <fx:root prefHeight="200.0" prefWidth="600.0" type="javafx.scene.layout.GridPane" xmlns="http://javafx.com/javafx/8.0.171" xmlns:fx="http://javafx.com/fxml/1" fx:controller="org.jabref.gui.SaveOrderConfigDisplayView">
     <columnConstraints>
         <ColumnConstraints hgrow="SOMETIMES" minWidth="-Infinity" prefWidth="100.0" />

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

@@ -6,23 +6,19 @@
 import java.util.stream.Collectors;
 
 import javafx.beans.binding.Bindings;
-import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.ReadOnlyListProperty;
 import javafx.beans.property.ReadOnlyListWrapper;
-import javafx.beans.property.SimpleObjectProperty;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
 import javafx.collections.ObservableMap;
 
+import org.jabref.gui.util.OptionalObjectProperty;
 import org.jabref.logic.search.SearchQuery;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.groups.GroupTreeNode;
 import org.jabref.model.util.OptionalUtil;
 
-import org.fxmisc.easybind.EasyBind;
-import org.fxmisc.easybind.monadic.MonadicBinding;
-
 /**
  * This class manages the GUI-state of JabRef, including:
  * - currently selected database
@@ -33,22 +29,21 @@
  */
 public class StateManager {
 
-    private final ObjectProperty<Optional<BibDatabaseContext>> activeDatabase = new SimpleObjectProperty<>(Optional.empty());
+    private final OptionalObjectProperty<BibDatabaseContext> activeDatabase = OptionalObjectProperty.empty();
     private final ReadOnlyListWrapper<GroupTreeNode> activeGroups = new ReadOnlyListWrapper<>(FXCollections.observableArrayList());
     private final ObservableList<BibEntry> selectedEntries = FXCollections.observableArrayList();
     private final ObservableMap<BibDatabaseContext, ObservableList<GroupTreeNode>> selectedGroups = FXCollections.observableHashMap();
-    private final ObjectProperty<Optional<SearchQuery>> activeSearchQuery = new SimpleObjectProperty<>(Optional.empty());
+    private final OptionalObjectProperty<SearchQuery> activeSearchQuery = OptionalObjectProperty.empty();
 
     public StateManager() {
-        MonadicBinding<BibDatabaseContext> currentDatabase = EasyBind.map(activeDatabase, database -> database.orElse(null));
-        activeGroups.bind(Bindings.valueAt(selectedGroups, currentDatabase));
+        activeGroups.bind(Bindings.valueAt(selectedGroups, activeDatabase.orElse(null)));
     }
 
-    public ObjectProperty<Optional<BibDatabaseContext>> activeDatabaseProperty() {
+    public OptionalObjectProperty<BibDatabaseContext> activeDatabaseProperty() {
         return activeDatabase;
     }
 
-    public ObjectProperty<Optional<SearchQuery>> activeSearchQueryProperty() {
+    public OptionalObjectProperty<SearchQuery> activeSearchQueryProperty() {
         return activeSearchQuery;
     }
 

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

@@ -1,24 +1,35 @@
 package org.jabref.gui.actions;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.Objects;
 
 import javafx.scene.Node;
 import javafx.scene.control.Button;
 import javafx.scene.control.ButtonBase;
 import javafx.scene.control.CheckMenuItem;
+import javafx.scene.control.Label;
 import javafx.scene.control.Menu;
 import javafx.scene.control.MenuItem;
+import javafx.scene.control.Tooltip;
 
 import org.jabref.gui.keyboard.KeyBindingRepository;
+import org.jabref.model.strings.StringUtil;
 
+import com.sun.javafx.scene.control.skin.ContextMenuContent;
 import de.saxsys.mvvmfx.utils.commands.Command;
 import org.controlsfx.control.action.ActionUtils;
+import org.fxmisc.easybind.EasyBind;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Helper class to create and style controls according to an {@link Action}.
  */
 public class ActionFactory {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(ActionFactory.class);
+
     private final KeyBindingRepository keyBindingRepository;
 
     public ActionFactory(KeyBindingRepository keyBindingRepository) {
@@ -33,14 +44,64 @@ private static void setGraphic(MenuItem node, Action action) {
         action.getIcon().ifPresent(icon -> node.setGraphic(icon.getGraphicNode()));
     }
 
-    public MenuItem configureMenuItem(Action action, Command command, MenuItem menuItem) {
-        return ActionUtils.configureMenuItem(new JabRefAction(action, command, keyBindingRepository), menuItem);
+    /*
+     * Returns MenuItemContainer node associated with this menu item
+     * which can contain:
+     *   1. label node of type Label for displaying menu item text,
+     *   2. right node of type Label for displaying accelerator text,
+     *      or an arrow if it's a Menu,
+     *   3. graphic node for displaying menu item icon, and
+     *   4. left node for displaying either radio button or check box.
+     *
+     * This is basically rewritten impl_styleableGetNode() which
+     * should not be used since it's marked as deprecated.
+     */
+    private static Label getAssociatedNode(MenuItem menuItem) {
+        ContextMenuContent.MenuItemContainer container = (ContextMenuContent.MenuItemContainer) menuItem.impl_styleableGetNode();
+
+        if (container == null) {
+            return null;
+        } else {
+            // We have to use reflection to get the associated label
+            try {
+                Method getLabel = ContextMenuContent.MenuItemContainer.class.getDeclaredMethod("getLabel");
+                getLabel.setAccessible(true);
+                return (Label) getLabel.invoke(container);
+            } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
+                LOGGER.warn("Could not get label of menu item", e);
+            }
+        }
+        return null;
     }
 
-    public MenuItem createMenuItem(Action action, Command command) {
-        MenuItem menuItem = ActionUtils.createMenuItem(new JabRefAction(action, command, keyBindingRepository));
+    public MenuItem configureMenuItem(Action action, Command command, MenuItem menuItem) {
+        ActionUtils.configureMenuItem(new JabRefAction(action, command, keyBindingRepository), menuItem);
         setGraphic(menuItem, action);
 
+        // Show tooltips
+        if (command instanceof SimpleCommand) {
+            EasyBind.subscribe(
+                    ((SimpleCommand) command).statusMessageProperty(),
+                    message -> {
+                        Label label = getAssociatedNode(menuItem);
+                        if (label != null) {
+                            label.setMouseTransparent(false);
+                            if (StringUtil.isBlank(message)) {
+                                label.setTooltip(null);
+                            } else {
+                                label.setTooltip(new Tooltip(message));
+                            }
+                        }
+                    }
+            );
+        }
+
+        return menuItem;
+    }
+
+    public MenuItem createMenuItem(Action action, Command command) {
+        MenuItem menuItem = new MenuItem();
+        configureMenuItem(action, command, menuItem);
         return menuItem;
     }
 

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

@@ -0,0 +1,16 @@
+package org.jabref.gui.actions;
+
+import javafx.beans.binding.Bindings;
+import javafx.beans.binding.BooleanExpression;
+
+import org.jabref.gui.StateManager;
+
+public class ActionHelper {
+    public static BooleanExpression needsDatabase(StateManager stateManager) {
+        return stateManager.activeDatabaseProperty().isPresent();
+    }
+
+    public static BooleanExpression needsEntriesSelected(StateManager stateManager) {
+        return Bindings.isNotEmpty(stateManager.getSelectedEntries());
+    }
+}

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

@@ -3,6 +3,7 @@
 /**
  * Global String constants for GUI actions
  */
+@Deprecated
 public enum Actions {
 
     ABBREVIATE_ISO,

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

@@ -4,8 +4,11 @@
  * BaseAction is used to define actions that are called from the
  * base frame through runCommand(). runCommand() finds the
  * appropriate BaseAction object, and runs its action() method.
+ *
+ * @deprecated use {@link SimpleCommand} instead
  */
 @FunctionalInterface
+@Deprecated
 public interface BaseAction {
 
     void action() throws Exception;

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

@@ -1,5 +1,7 @@
 package org.jabref.gui.actions;
 
+import javafx.beans.binding.Bindings;
+
 import org.jabref.Globals;
 import org.jabref.gui.keyboard.KeyBindingRepository;
 
@@ -10,7 +12,6 @@
  */
 class JabRefAction extends org.controlsfx.control.action.Action {
 
-
     public JabRefAction(Action action, KeyBindingRepository keyBindingRepository) {
         super(action.getText());
         action.getIcon()
@@ -19,22 +20,34 @@ public JabRefAction(Action action, KeyBindingRepository keyBindingRepository) {
               .ifPresent(keyBinding -> setAccelerator(keyBindingRepository.getKeyCombination(keyBinding)));
 
         setLongText(action.getDescription());
-
     }
 
     public JabRefAction(Action action, Command command, KeyBindingRepository keyBindingRepository) {
         this(action, keyBindingRepository);
 
         setEventHandler(event -> {
             command.execute();
-            trackExecute();
+            trackExecute(getActionName(action, command));
         });
 
         disabledProperty().bind(command.executableProperty().not());
+
+        if (command instanceof SimpleCommand) {
+            SimpleCommand ourCommand = (SimpleCommand) command;
+            longTextProperty().bind(Bindings.concat(action.getDescription(), ourCommand.statusMessageProperty()));
+        }
+    }
+
+    private String getActionName(Action action, Command command) {
+        if (command.getClass().isAnonymousClass()) {
+            return action.getText();
+        } else {
+            return command.getClass().getSimpleName();
+        }
     }
 
-    private void trackExecute() {
+    private void trackExecute(String actionName) {
         Globals.getTelemetryClient()
-               .ifPresent(telemetryClient -> telemetryClient.trackEvent(getText()));
+               .ifPresent(telemetryClient -> telemetryClient.trackEvent(actionName));
     }
 }

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

@@ -13,6 +13,7 @@
  * This wraps the old Swing commands so that they fit into the new infrastructure.
  * In the long term, this class should be removed.
  */
+@Deprecated
 public class OldCommandWrapper extends CommandBase {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(OldCommandWrapper.class);

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

@@ -13,6 +13,7 @@
  * This wraps the old Swing commands so that they fit into the new infrastructure.
  * In the long term, this class should be removed.
  */
+@Deprecated
 public class OldCommandWrapperForActiveDatabase extends CommandBase {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(OldCommandWrapperForActiveDatabase.class);