Skip to content

Commit

Permalink
Fix Copy cite command should respect preferences (JabRef#10707)
Browse files Browse the repository at this point in the history
* Fix Copy cite command should respect preferences

Fixes JabRef#10615

* add changelog
fix checsktyle

* Fix CHANGELOG.md

* Add missing "citation" to "key"

* Add debug message at error in logic in CopyMoreAction

* Add fallback if preference could not be parsed

* Fix key binding setting

* More relaxed parsing

* Streamline code in AbstractPushToApplication

* Add more test cases for configured citation command

* Streamline code for copy action

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

* Fix test

---------

Co-authored-by: Oliver Kopp <[email protected]>
  • Loading branch information
Siedlerchr and koppor authored Dec 22, 2023
1 parent 6014172 commit 11d90a1
Show file tree
Hide file tree
Showing 13 changed files with 174 additions and 113 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/actions/StandardActions.java
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
97 changes: 46 additions & 51 deletions src/main/java/org/jabref/gui/edit/CopyMoreAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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.");
}
}

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

Expand All @@ -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.
Expand All @@ -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();

Expand All @@ -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)));
}
}

Expand Down Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/org/jabref/gui/keyboard/KeyBinding.java
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand Down Expand Up @@ -229,6 +231,6 @@ public StringProperty customFileBrowserCommandProperty() {
}

public void resetCiteCommandToDefault() {
this.citeCommandProperty.setValue(preferences.getExternalApplicationsPreferences().getDefaultCiteCommand());
this.citeCommandProperty.setValue(preferences.getExternalApplicationsPreferences().getDefaultCiteCommand().toString());
}
}
37 changes: 3 additions & 34 deletions src/main/java/org/jabref/gui/push/AbstractPushToApplication.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions src/main/java/org/jabref/logic/push/CitationCommandString.java
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading

0 comments on commit 11d90a1

Please sign in to comment.