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

* upstream/master:
  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

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);