Fix Copy cite command should respect preferences

CHANGELOG.md

@@ -29,6 +29,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - We upgraded to JavaFX 21.0.1. As a consequence JabRef requires now macOS 11 or later and GTK 3.8 or later on Linux [10627](https://github.com/JabRef/jabref/pull/10627).
 - A user-specific comment fields is not enabled by default, but can be enabled using the "Add" button. [#10424](https://github.com/JabRef/jabref/issues/10424)
 - We upgraded to Lucene 9.9 for the fulltext search. The search index will be rebuild. [#10686](https://github.com/JabRef/jabref/pull/10686)
+- When using "Copy..." -> "Copy citation key", the delimiter configured at "Push applications" is respected. [#10707](https://github.com/JabRef/jabref/pull/10707)
 
 ### Fixed
 
@@ -37,6 +38,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - We fixed some small inconsistencies in the user interface. [#10507](https://github.com/JabRef/jabref/issues/10507) [#10458](https://github.com/JabRef/jabref/issues/10458) [#10660](https://github.com/JabRef/jabref/issues/10660)
 - We fixed the issue where the Hayagriva YAML exporter would not include a parent field for the publisher/series. [#10596](https://github.com/JabRef/jabref/issues/10596)
 - We fixed issues in the external file type dialog w.r.t. duplicate entries in the case of a language switch. [#10271](https://github.com/JabRef/jabref/issues/10271)
+- We fixed an issue where the right-click action "Copy cite..." did not respect the configured citation command under "External Programs" -> "[Push Applications](https://docs.jabref.org/cite/pushtoapplications)" [#10615](https://github.com/JabRef/jabref/issues/10615)
 
 ### Removed
 

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

@@ -13,7 +13,7 @@ public enum StandardActions implements Action {
     COPY_MORE(Localization.lang("Copy") + "..."),
     COPY_TITLE(Localization.lang("Copy title"), KeyBinding.COPY_TITLE),
     COPY_KEY(Localization.lang("Copy citation key"), KeyBinding.COPY_CITATION_KEY),
-    COPY_CITE_KEY(Localization.lang("Copy \\cite{citation key}"), KeyBinding.COPY_CITE_CITATION_KEY),
+    COPY_CITE_KEY(Localization.lang("Copy citation key with configured cite command"), KeyBinding.COPY_CITE_CITATION_KEY),
     COPY_KEY_AND_TITLE(Localization.lang("Copy citation key and title"), KeyBinding.COPY_CITATION_KEY_AND_TITLE),
     COPY_KEY_AND_LINK(Localization.lang("Copy citation key and link"), KeyBinding.COPY_CITATION_KEY_AND_LINK),
     COPY_CITATION_HTML(Localization.lang("Copy citation (html)"), KeyBinding.COPY_PREVIEW),

src/main/java/org/jabref/gui/edit/CopyMoreAction.java

@@ -3,7 +3,7 @@
 import java.io.IOException;
 import java.io.StringReader;
 import java.util.List;
-import java.util.Optional;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import org.jabref.gui.ClipBoardManager;
@@ -17,7 +17,9 @@
 import org.jabref.logic.l10n.Localization;
 import org.jabref.logic.layout.Layout;
 import org.jabref.logic.layout.LayoutHelper;
+import org.jabref.logic.push.CitationCommandString;
 import org.jabref.logic.util.OS;
+import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.entry.field.StandardField;
 import org.jabref.preferences.PreferencesService;
@@ -58,13 +60,20 @@ public void execute() {
         }
 
         switch (action) {
-            case COPY_TITLE -> copyTitle();
-            case COPY_KEY -> copyKey();
-            case COPY_CITE_KEY -> copyCiteKey();
-            case COPY_KEY_AND_TITLE -> copyKeyAndTitle();
-            case COPY_KEY_AND_LINK -> copyKeyAndLink();
-            case COPY_DOI, COPY_DOI_URL -> copyDoi();
-            default -> LOGGER.info("Unknown copy command.");
+            case COPY_TITLE ->
+                    copyTitle();
+            case COPY_KEY ->
+                    copyKey();
+            case COPY_CITE_KEY ->
+                    copyCiteKey();
+            case COPY_KEY_AND_TITLE ->
+                    copyKeyAndTitle();
+            case COPY_KEY_AND_LINK ->
+                    copyKeyAndLink();
+            case COPY_DOI, COPY_DOI_URL ->
+                    copyDoi();
+            default ->
+                    LOGGER.info("Unknown copy command.");
         }
     }
 
@@ -94,47 +103,20 @@ private void copyTitle() {
         }
     }
 
-    private void copyKey() {
-        List<BibEntry> entries = stateManager.getSelectedEntries();
-
-        // Collect all non-null keys.
-        List<String> keys = entries.stream()
-                                   .filter(entry -> entry.getCitationKey().isPresent())
-                                   .map(entry -> entry.getCitationKey().get())
-                                   .collect(Collectors.toList());
-
-        if (keys.isEmpty()) {
-            dialogService.notify(Localization.lang("None of the selected entries have citation keys."));
-            return;
-        }
-
-        final String copiedKeys = String.join(",", keys);
-        clipBoardManager.setContent(copiedKeys);
-
-        if (keys.size() == entries.size()) {
-            // All entries had keys.
-            dialogService.notify(Localization.lang("Copied '%0' to clipboard.",
-                    JabRefDialogService.shortenDialogMessage(copiedKeys)));
-        } else {
-            dialogService.notify(Localization.lang("Warning: %0 out of %1 entries have undefined citation key.",
-                    Integer.toString(entries.size() - keys.size()), Integer.toString(entries.size())));
-        }
-    }
-
     private void copyDoi() {
         List<BibEntry> entries = stateManager.getSelectedEntries();
 
         // Collect all non-null DOI or DOI urls
         if (action == StandardActions.COPY_DOI_URL) {
             copyDoiList(entries.stream()
-                    .filter(entry -> entry.getDOI().isPresent())
-                    .map(entry -> entry.getDOI().get().getURIAsASCIIString())
-                    .collect(Collectors.toList()), entries.size());
+                               .filter(entry -> entry.getDOI().isPresent())
+                               .map(entry -> entry.getDOI().get().getURIAsASCIIString())
+                               .collect(Collectors.toList()), entries.size());
         } else {
             copyDoiList(entries.stream()
-                    .filter(entry -> entry.getDOI().isPresent())
-                    .map(entry -> entry.getDOI().get().getDOI())
-                    .collect(Collectors.toList()), entries.size());
+                               .filter(entry -> entry.getDOI().isPresent())
+                               .map(entry -> entry.getDOI().get().getDOI())
+                               .collect(Collectors.toList()), entries.size());
         }
     }
 
@@ -157,7 +139,7 @@ private void copyDoiList(List<String> dois, int size) {
         }
     }
 
-    private void copyCiteKey() {
+    private void doCopyKey(Function<List<String>, String> mapKeyList) {
         List<BibEntry> entries = stateManager.getSelectedEntries();
 
         // Collect all non-null keys.
@@ -171,23 +153,31 @@ private void copyCiteKey() {
             return;
         }
 
-        String citeCommand = Optional.ofNullable(preferencesService.getExternalApplicationsPreferences().getCiteCommand())
-                                     .filter(cite -> cite.contains("\\")) // must contain \
-                                     .orElse("\\cite");
+        String clipBoardContent = mapKeyList.apply(keys);
 
-        final String copiedCiteCommand = citeCommand + "{" + String.join(",", keys) + '}';
-        clipBoardManager.setContent(copiedCiteCommand);
+        clipBoardManager.setContent(clipBoardContent);
 
         if (keys.size() == entries.size()) {
             // All entries had keys.
             dialogService.notify(Localization.lang("Copied '%0' to clipboard.",
-                    JabRefDialogService.shortenDialogMessage(copiedCiteCommand)));
+                    JabRefDialogService.shortenDialogMessage(clipBoardContent)));
         } else {
             dialogService.notify(Localization.lang("Warning: %0 out of %1 entries have undefined citation key.",
                     Integer.toString(entries.size() - keys.size()), Integer.toString(entries.size())));
         }
     }
 
+    private void copyCiteKey() {
+        doCopyKey(keys -> {
+            CitationCommandString citeCommand = preferencesService.getExternalApplicationsPreferences().getCiteCommand();
+            return citeCommand.prefix() + String.join(citeCommand.delimiter(), keys) + citeCommand.suffix();
+        });
+    }
+
+    private void copyKey() {
+        doCopyKey(keys -> String.join(preferencesService.getExternalApplicationsPreferences().getCiteCommand().delimiter(), keys));
+    }
+
     private void copyKeyAndTitle() {
         List<BibEntry> entries = stateManager.getSelectedEntries();
 
@@ -208,7 +198,9 @@ private void copyKeyAndTitle() {
         for (BibEntry entry : entries) {
             if (entry.hasCitationKey()) {
                 entriesWithKeys++;
-                keyAndTitle.append(layout.doLayout(entry, stateManager.getActiveDatabase().get().getDatabase()));
+                stateManager.getActiveDatabase()
+                            .map(BibDatabaseContext::getDatabase)
+                            .ifPresent(bibDatabase -> keyAndTitle.append(layout.doLayout(entry, bibDatabase)));
             }
         }
 
@@ -242,15 +234,18 @@ private void copyKeyAndLink() {
 
         List<BibEntry> entriesWithKey = entries.stream()
                                                .filter(BibEntry::hasCitationKey)
-                                               .collect(Collectors.toList());
+                                               .toList();
 
         if (entriesWithKey.isEmpty()) {
             dialogService.notify(Localization.lang("None of the selected entries have citation keys."));
             return;
         }
 
         for (BibEntry entry : entriesWithKey) {
-            String key = entry.getCitationKey().get();
+            String key = entry.getCitationKey().orElse("");
+            if (LOGGER.isDebugEnabled() && key.isEmpty()) {
+                LOGGER.debug("entry {} had no citation key, but it should have had one", entry);
+            }
             String url = entry.getField(StandardField.URL).orElse("");
             keyAndLink.append(url.isEmpty() ? key : String.format("<a href=\"%s\">%s</a>", url, key));
             keyAndLink.append(OS.NEWLINE);

src/main/java/org/jabref/gui/keyboard/KeyBinding.java

@@ -32,7 +32,10 @@ public enum KeyBinding {
     CLOSE("Close dialog", Localization.lang("Close dialog"), "Esc", KeyBindingCategory.VIEW),
     COPY("Copy", Localization.lang("Copy"), "ctrl+C", KeyBindingCategory.EDIT),
     COPY_TITLE("Copy title", Localization.lang("Copy title"), "ctrl+shift+alt+T", KeyBindingCategory.EDIT),
-    COPY_CITE_CITATION_KEY("Copy \\cite{citation key}", Localization.lang("Copy \\cite{citation key}"), "ctrl+K", KeyBindingCategory.EDIT),
+
+    // We migrated from "Copy \\cite{citation key}" to "Copy citation key with configured cite command", therefore we keep the "old string" for backwards comppatibility
+    COPY_CITE_CITATION_KEY("Copy \\cite{citation key}", Localization.lang("Copy citation key with configured cite command"), "ctrl+K", KeyBindingCategory.EDIT),
+
     COPY_CITATION_KEY("Copy citation key", Localization.lang("Copy citation key"), "ctrl+shift+K", KeyBindingCategory.EDIT),
     COPY_CITATION_KEY_AND_TITLE("Copy citation key and title", Localization.lang("Copy citation key and title"), "ctrl+shift+alt+K", KeyBindingCategory.EDIT),
     COPY_CITATION_KEY_AND_LINK("Copy citation key and link", Localization.lang("Copy citation key and link"), "ctrl+alt+K", KeyBindingCategory.EDIT),

src/main/java/org/jabref/gui/preferences/external/ExternalTabViewModel.java

@@ -22,6 +22,7 @@
 import org.jabref.gui.push.PushToEmacs;
 import org.jabref.gui.util.FileDialogConfiguration;
 import org.jabref.logic.l10n.Localization;
+import org.jabref.logic.push.CitationCommandString;
 import org.jabref.model.strings.StringUtil;
 import org.jabref.preferences.ExternalApplicationsPreferences;
 import org.jabref.preferences.PreferencesService;
@@ -39,7 +40,7 @@ public class ExternalTabViewModel implements PreferenceTabViewModel {
     private final BooleanProperty autoOpenAttachedFoldersProperty = new SimpleBooleanProperty();
     private final ListProperty<PushToApplication> pushToApplicationsListProperty = new SimpleListProperty<>();
     private final ObjectProperty<PushToApplication> selectedPushToApplicationProperty = new SimpleObjectProperty<>();
-    private final StringProperty citeCommandProperty = new SimpleStringProperty("");
+    private final StringProperty citeCommandProperty = new SimpleStringProperty();
     private final BooleanProperty useCustomTerminalProperty = new SimpleBooleanProperty();
     private final StringProperty customTerminalCommandProperty = new SimpleStringProperty("");
     private final BooleanProperty useCustomFileBrowserProperty = new SimpleBooleanProperty();
@@ -97,7 +98,8 @@ public void setValues() {
                 PushToApplications.getApplicationByName(initialPushToApplicationPreferences.getActiveApplicationName(), dialogService, preferences)
                                   .orElse(new PushToEmacs(dialogService, preferences)));
 
-        citeCommandProperty.setValue(initialExternalApplicationPreferences.getCiteCommand());
+        citeCommandProperty.setValue(initialExternalApplicationPreferences.getCiteCommand().toString());
+
         useCustomTerminalProperty.setValue(initialExternalApplicationPreferences.useCustomTerminal());
         customTerminalCommandProperty.setValue(initialExternalApplicationPreferences.getCustomTerminalCommand());
         useCustomFileBrowserProperty.setValue(initialExternalApplicationPreferences.useCustomFileBrowser());
@@ -110,7 +112,7 @@ public void storeSettings() {
         ExternalApplicationsPreferences externalPreferences = preferences.getExternalApplicationsPreferences();
         externalPreferences.setEMailSubject(eMailReferenceSubjectProperty.getValue());
         externalPreferences.setAutoOpenEmailAttachmentsFolder(autoOpenAttachedFoldersProperty.getValue());
-        externalPreferences.setCiteCommand(citeCommandProperty.getValue());
+        externalPreferences.setCiteCommand(CitationCommandString.from(citeCommandProperty.getValue()));
         externalPreferences.setUseCustomTerminal(useCustomTerminalProperty.getValue());
         externalPreferences.setCustomTerminalCommand(customTerminalCommandProperty.getValue());
         externalPreferences.setUseCustomFileBrowser(useCustomFileBrowserProperty.getValue());
@@ -229,6 +231,6 @@ public StringProperty customFileBrowserCommandProperty() {
     }
 
     public void resetCiteCommandToDefault() {
-        this.citeCommandProperty.setValue(preferences.getExternalApplicationsPreferences().getDefaultCiteCommand());
+        this.citeCommandProperty.setValue(preferences.getExternalApplicationsPreferences().getDefaultCiteCommand().toString());
     }
 }

src/main/java/org/jabref/gui/push/AbstractPushToApplication.java

@@ -26,9 +26,6 @@
 public abstract class AbstractPushToApplication implements PushToApplication {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(AbstractPushToApplication.class);
-    private static final String CITE_KEY1 = "key1";
-    private static final String CITE_KEY2 = "key2";
-
     protected boolean couldNotCall; // Set to true in case the command could not be executed, e.g., if the file is not found
     protected boolean couldNotPush; // Set to true in case the tunnel to the program (if one is used) does not operate
     protected boolean notDefined; // Set to true if the corresponding path is not defined in the preferences
@@ -38,36 +35,11 @@ public abstract class AbstractPushToApplication implements PushToApplication {
     protected final DialogService dialogService;
     protected final PreferencesService preferencesService;
 
-    private String cachedCiteCommand;
-    private String cachedCitePrefix;
-    private String cachedCiteSuffix;
-    private String cachedCiteDelimiter;
-
     public AbstractPushToApplication(DialogService dialogService, PreferencesService preferencesService) {
         this.dialogService = dialogService;
         this.preferencesService = preferencesService;
     }
 
-    private void dissectCiteCommand() {
-        String preferencesCiteCommand = preferencesService.getExternalApplicationsPreferences().getCiteCommand();
-
-        if (preferencesCiteCommand != null && preferencesCiteCommand.equals(cachedCiteCommand)) {
-            return;
-        }
-
-        cachedCiteCommand = preferencesCiteCommand;
-
-        int indexKey1 = cachedCiteCommand.indexOf(CITE_KEY1);
-        int indexKey2 = cachedCiteCommand.indexOf(CITE_KEY2);
-        if (indexKey1 < 0 || indexKey2 < 0 || indexKey2 < (indexKey1 + CITE_KEY1.length())) {
-            return;
-        }
-
-        cachedCitePrefix = preferencesCiteCommand.substring(0, indexKey1);
-        cachedCiteDelimiter = preferencesCiteCommand.substring(preferencesCiteCommand.lastIndexOf(CITE_KEY1) + CITE_KEY1.length(), indexKey2);
-        cachedCiteSuffix = preferencesCiteCommand.substring(preferencesCiteCommand.lastIndexOf(CITE_KEY2) + CITE_KEY2.length());
-    }
-
     @Override
     public JabRefIcon getApplicationIcon() {
         return IconTheme.JabRefIcons.APPLICATION_GENERIC;
@@ -170,18 +142,15 @@ protected String getCommandName() {
     }
 
     protected String getCitePrefix() {
-        dissectCiteCommand();
-        return cachedCitePrefix;
+        return preferencesService.getExternalApplicationsPreferences().getCiteCommand().prefix();
     }
 
     public String getDelimiter() {
-        dissectCiteCommand();
-        return cachedCiteDelimiter;
+        return preferencesService.getExternalApplicationsPreferences().getCiteCommand().delimiter();
     }
 
     protected String getCiteSuffix() {
-        dissectCiteCommand();
-        return cachedCiteSuffix;
+        return preferencesService.getExternalApplicationsPreferences().getCiteCommand().suffix();
     }
 
     @Override

src/main/java/org/jabref/logic/push/CitationCommandString.java

@@ -0,0 +1,34 @@
+package org.jabref.logic.push;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public record CitationCommandString(String prefix, String delimiter, String suffix) {
+    private static final Logger LOGGER = LoggerFactory.getLogger(CitationCommandString.class);
+    private static final String CITE_KEY1 = "key1";
+    private static final String CITE_KEY2 = "key2";
+
+    @Override
+    public String toString() {
+        return prefix + CITE_KEY1 + delimiter + CITE_KEY2 + suffix;
+    }
+
+    public static CitationCommandString from(String completeCiteCommand) {
+        int indexKey1 = completeCiteCommand.indexOf(CITE_KEY1);
+        int indexKey2 = completeCiteCommand.indexOf(CITE_KEY2);
+        if (indexKey1 < 0 || indexKey2 < 0 || indexKey2 < (indexKey1 + CITE_KEY1.length())) {
+            LOGGER.info("Wrong indexes {} {} for completeCiteCommand {}. Using default delimiter and suffix.", indexKey1, indexKey2, completeCiteCommand);
+            if (completeCiteCommand.isEmpty()) {
+                completeCiteCommand = "\\cite{";
+            } else if (!completeCiteCommand.endsWith("{")) {
+                completeCiteCommand += "{";
+            }
+            return new CitationCommandString(completeCiteCommand, ",", "}");
+        }
+
+        String prefix = completeCiteCommand.substring(0, indexKey1);
+        String delim = completeCiteCommand.substring(completeCiteCommand.lastIndexOf(CITE_KEY1) + CITE_KEY1.length(), indexKey2);
+        String suffix = completeCiteCommand.substring(completeCiteCommand.lastIndexOf(CITE_KEY2) + CITE_KEY2.length());
+        return new CitationCommandString(prefix, delim, suffix);
+    }
+}