Push to external application config (#10303)

* Introduce SelfContainedSaveConfiguration

* prepare adding cite key delimiter and start end chars

* Fix checkstyle

* Fix NPE

* add preferences and text fields

* fix sublime escaping

* fix emacs

* add

* fix checkstyle

* fix imports

* checkstyle

* checkstyle the xt

* Update JabRef_en.properties

* Introduced dissectCiteCommand

* Fixed compile

* fix cite key and add error logger

* fix npe

* fix

* fix index of

* Update JabRef_en.properties

fix l10n

* fix texstudio insert

* Shorten localization strings for emacs (and group the pushing localization)

* Rename "couldNotConnect" to "couldNotPush"

* Some debug

* New variable pushToApplicationPreferences

* Works on Windows

* Use "replace" instead of "replaceAll"

* Shorten messages for Vim

* Add logger to Vim execution, too

* Style: LOGGER first

* Modified test for linux

* Make emacs test "portable"

* Fix localization language

---------

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>

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

@@ -123,9 +123,9 @@ public JabRefFrame(Stage mainStage) {
  this.fileUpdateMonitor = Globals.getFileUpdateMonitor();
  this.entryTypesManager = Globals.entryTypesManager;
  this.globalSearchBar = new GlobalSearchBar(this, stateManager, prefs, undoManager, dialogService);
- this.pushToApplicationCommand = new PushToApplicationCommand(stateManager, dialogService, prefs);
- this.fileHistory = new FileHistoryMenu(prefs.getGuiPreferences().getFileHistory(), dialogService, getOpenDatabaseAction());
  this.taskExecutor = Globals.TASK_EXECUTOR;
+ this.pushToApplicationCommand = new PushToApplicationCommand(stateManager, dialogService, prefs, taskExecutor);
+ this.fileHistory = new FileHistoryMenu(prefs.getGuiPreferences().getFileHistory(), dialogService, getOpenDatabaseAction());
  this.setOnKeyTyped(key -> {
  if (this.fileHistory.isShowing()) {
  if (this.fileHistory.openFileByKey(key)) {

src/main/java/org/jabref/gui/preferences/external/ExternalTab.fxml

@@ -55,7 +55,7 @@
 
  <Label text="%Cite command" GridPane.rowIndex="2"/>
  <TextField fx:id="citeCommand"
- prefWidth="200.0" GridPane.columnIndex="1" GridPane.rowIndex="2"/>
+ prefWidth="300.0" GridPane.columnIndex="1" GridPane.rowIndex="2"/>
  </GridPane>
  </HBox>
 

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

@@ -23,6 +23,7 @@ public class ExternalTab extends AbstractPreferenceTabView<ExternalTabViewModel>
  @FXML private CheckBox autoOpenAttachedFolders;
  @FXML private ComboBox<PushToApplication> pushToApplicationCombo;
  @FXML private TextField citeCommand;
+
  @FXML private CheckBox useCustomTerminal;
  @FXML private TextField customTerminalCommand;
  @FXML private Button customTerminalBrowse;

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

@@ -210,8 +210,6 @@ public StringProperty citeCommandProperty() {
  return this.citeCommandProperty;
  }
 
- // Open console
-
  public BooleanProperty useCustomTerminalProperty() {
  return this.useCustomTerminalProperty;
  }

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

@@ -26,21 +26,48 @@
 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 couldNotConnect; // Set to true in case the tunnel to the program (if one is used) does not operate
+ 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
 
  protected String commandPath;
 
  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;
@@ -58,7 +85,7 @@ public Action getAction() {
 
  @Override
  public void pushEntries(BibDatabaseContext database, List<BibEntry> entries, String keyString) {
- couldNotConnect = false;
+ couldNotPush = false;
  couldNotCall = false;
  notDefined = false;
 
@@ -108,7 +135,7 @@ public void onOperationCompleted() {
  dialogService.showErrorDialogAndWait(
  Localization.lang("Error pushing entries"),
  Localization.lang("Could not call executable") + " '" + commandPath + "'.");
- } else if (couldNotConnect) {
+ } else if (couldNotPush) {
  dialogService.showErrorDialogAndWait(
  Localization.lang("Error pushing entries"),
  Localization.lang("Could not connect to %0", getDisplayName()) + ".");
@@ -142,8 +169,19 @@ protected String getCommandName() {
  return null;
  }
 
- protected String getCiteCommand() {
- return preferencesService.getExternalApplicationsPreferences().getCiteCommand();
+ protected String getCitePrefix() {
+ dissectCiteCommand();
+ return cachedCitePrefix;
+ }
+
+ public String getDelimiter() {
+ dissectCiteCommand();
+ return cachedCiteDelimiter;
+ }
+
+ protected String getCiteSuffix() {
+ dissectCiteCommand();
+ return cachedCiteSuffix;
  }
 
  @Override

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

@@ -41,4 +41,6 @@ public interface PushToApplication {
  Action getAction();
 
  PushToApplicationSettings getSettings(PushToApplication application, PushToApplicationPreferences pushToApplicationPreferences);
+
+ String getDelimiter();
 }

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

@@ -9,13 +9,13 @@
 import javafx.scene.control.MenuItem;
 
 import org.jabref.gui.DialogService;
-import org.jabref.gui.Globals;
 import org.jabref.gui.StateManager;
 import org.jabref.gui.actions.Action;
 import org.jabref.gui.actions.ActionFactory;
 import org.jabref.gui.actions.SimpleCommand;
 import org.jabref.gui.util.BackgroundTask;
 import org.jabref.gui.util.BindingsHelper;
+import org.jabref.gui.util.TaskExecutor;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
@@ -41,13 +41,15 @@ public class PushToApplicationCommand extends SimpleCommand {
  private final PreferencesService preferencesService;
 
  private final List<Object> reconfigurableControls = new ArrayList<>();
+ private final TaskExecutor taskExecutor;
 
  private PushToApplication application;
 
- public PushToApplicationCommand(StateManager stateManager, DialogService dialogService, PreferencesService preferencesService) {
+ public PushToApplicationCommand(StateManager stateManager, DialogService dialogService, PreferencesService preferencesService, TaskExecutor taskExecutor) {
  this.stateManager = stateManager;
  this.dialogService = dialogService;
  this.preferencesService = preferencesService;
+ this.taskExecutor = taskExecutor;
 
  setApplication(preferencesService.getPushToApplicationPreferences()
  .getActiveApplicationName());
@@ -95,7 +97,7 @@ public Action getAction() {
  return application.getAction();
  }
 
- private static String getKeyString(List<BibEntry> entries) {
+ private static String getKeyString(List<BibEntry> entries, String delimiter) {
  StringBuilder result = new StringBuilder();
  Optional<String> citeKey;
  boolean first = true;
@@ -109,7 +111,7 @@ private static String getKeyString(List<BibEntry> entries) {
  result.append(citeKey.get());
  first = false;
  } else {
- result.append(',').append(citeKey.get());
+ result.append(delimiter).append(citeKey.get());
  }
  }
  return result.toString();
@@ -132,11 +134,12 @@ public void execute() {
  // All set, call the operation in a new thread:
  BackgroundTask.wrap(this::pushEntries)
  .onSuccess(s -> application.onOperationCompleted())
- .executeWith(Globals.TASK_EXECUTOR);
+ .onFailure(ex -> LOGGER.error("Error pushing citation", ex))
+ .executeWith(taskExecutor);
  }
 
  private void pushEntries() {
  BibDatabaseContext database = stateManager.getActiveDatabase().orElseThrow(() -> new NullPointerException("Database null"));
- application.pushEntries(database, stateManager.getSelectedEntries(), getKeyString(stateManager.getSelectedEntries()));
+ application.pushEntries(database, stateManager.getSelectedEntries(), getKeyString(stateManager.getSelectedEntries(), application.getDelimiter()));
  }
 }

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

@@ -2,6 +2,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Arrays;
 import java.util.List;
 
 import org.jabref.gui.DialogService;
@@ -24,6 +25,9 @@ public class PushToEmacs extends AbstractPushToApplication {
 
  private static final Logger LOGGER = LoggerFactory.getLogger(PushToEmacs.class);
 
+ /**
+ * @param preferencesService getPushToApplicationPreferences(), getExternalApplicationsPreferences(), and getFilePreferences() are used
+ */
  public PushToEmacs(DialogService dialogService, PreferencesService preferencesService) {
  super(dialogService, preferencesService);
  }
@@ -40,43 +44,60 @@ public JabRefIcon getApplicationIcon() {
 
  @Override
  public void pushEntries(BibDatabaseContext database, List<BibEntry> entries, String keys) {
- couldNotConnect = false;
+ couldNotPush = false;
  couldNotCall = false;
  notDefined = false;
 
- commandPath = preferencesService.getPushToApplicationPreferences().getCommandPaths().get(this.getDisplayName());
+ PushToApplicationPreferences pushToApplicationPreferences = preferencesService.getPushToApplicationPreferences();
+
+ commandPath = pushToApplicationPreferences.getCommandPaths().get(this.getDisplayName());
 
  if ((commandPath == null) || commandPath.trim().isEmpty()) {
  notDefined = true;
  return;
  }
 
- commandPath = preferencesService.getPushToApplicationPreferences().getCommandPaths().get(this.getDisplayName());
+ commandPath = pushToApplicationPreferences.getCommandPaths().get(this.getDisplayName());
 
- String[] addParams = preferencesService.getPushToApplicationPreferences().getEmacsArguments().split(" ");
+ String[] addParams = pushToApplicationPreferences.getEmacsArguments().split(" ");
  try {
  String[] com = new String[addParams.length + 2];
  com[0] = commandPath;
  System.arraycopy(addParams, 0, com, 1, addParams.length);
- String prefix;
- String suffix;
- prefix = "(with-current-buffer (window-buffer) (insert ";
- suffix = "))";
+
+ // Surrounding with is handled below
+ String prefix = "(with-current-buffer (window-buffer (selected-window)) (insert ";
+ String suffix = "))";
 
  if (OS.WINDOWS) {
  // Windows gnuclient/emacsclient escaping:
  // java string: "(insert \\\"\\\\cite{Blah2001}\\\")";
  // so cmd receives: (insert \"\\cite{Blah2001}\")
  // so emacs receives: (insert "\cite{Blah2001}")
- com[com.length - 1] = prefix.concat("\\\"\\" + getCiteCommand().replaceAll("\\\\", "\\\\\\\\") + "{" + keys + "}\\\"").concat(suffix);
+
+ com[com.length - 1] = prefix.concat("\""
+ + getCitePrefix().replace("\\", "\\\\")
+ + keys
+ + getCiteSuffix().replace("\\", "\\\\")
+ + "\"").concat(suffix)
+ .replace("\"", "\\\"");
  } else {
  // Linux gnuclient/emacslient escaping:
  // java string: "(insert \"\\\\cite{Blah2001}\")"
  // so sh receives: (insert "\\cite{Blah2001}")
  // so emacs receives: (insert "\cite{Blah2001}")
- com[com.length - 1] = prefix.concat("\"" + getCiteCommand().replaceAll("\\\\", "\\\\\\\\") + "{" + keys + "}\"").concat(suffix);
+ com[com.length - 1] = prefix.concat("\""
+ + getCitePrefix().replace("\\", "\\\\")
+ + keys
+ + getCiteSuffix().replace("\\", "\\\\")
+ + "\"").concat(suffix);
  }
 
+ LOGGER.atDebug()
+ .setMessage("Executing command {}")
+ .addArgument(() -> Arrays.toString(com))
+ .log();
+
  final Process p = Runtime.getRuntime().exec(com);
 
  JabRefExecutorService.INSTANCE.executeAndWait(() -> {
@@ -92,30 +113,27 @@ public void pushEntries(BibDatabaseContext database, List<BibEntry> entries, Str
  }
  // Error stream has been closed. See if there were any errors:
  if (!sb.toString().trim().isEmpty()) {
- LOGGER.warn("Push to Emacs error: " + sb);
- couldNotConnect = true;
+ LOGGER.warn("Push to Emacs error: {}", sb);
+ couldNotPush = true;
  }
  } catch (IOException e) {
- LOGGER.warn("File problem.", e);
+ LOGGER.warn("Error handling std streams", e);
  }
  });
  } catch (IOException excep) {
- couldNotCall = true;
  LOGGER.warn("Problem pushing to Emacs.", excep);
+ couldNotCall = true;
  }
  }
 
  @Override
  public void onOperationCompleted() {
- if (couldNotConnect) {
+ if (couldNotPush) {
  dialogService.showErrorDialogAndWait(Localization.lang("Error pushing entries"),
- Localization.lang("Could not connect to a running gnuserv process. Make sure that "
- + "Emacs or XEmacs is running, and that the server has been started "
- + "(by running the command 'server-start'/'gnuserv-start')."));
+ Localization.lang("Could not push to a running emacs daemon."));
  } else if (couldNotCall) {
  dialogService.showErrorDialogAndWait(Localization.lang("Error pushing entries"),
- Localization.lang("Could not run the gnuclient/emacsclient program. Make sure you have "
- + "the emacsclient/gnuclient program installed and available in the PATH."));
+ Localization.lang("Could not run the emacs client."));
  } else {
  super.onOperationCompleted();
  }

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

@@ -42,7 +42,7 @@ public JabRefIcon getApplicationIcon() {
 
  @Override
  public void onOperationCompleted() {
- if (couldNotConnect) {
+ if (couldNotPush) {
  dialogService.showErrorDialogAndWait(Localization.lang("Error pushing entries"),
  Localization.lang("Verify that LyX is running and that the lyxpipe is valid.")
  + "[" + commandPath + "]");
@@ -60,7 +60,7 @@ public PushToApplicationSettings getSettings(PushToApplication application, Push
 
  @Override
  public void pushEntries(BibDatabaseContext database, final List<BibEntry> entries, final String keyString) {
- couldNotConnect = false;
+ couldNotPush = false;
  couldNotCall = false;
  notDefined = false;
 
@@ -79,7 +79,7 @@ public void pushEntries(BibDatabaseContext database, final List<BibEntry> entrie
  // See if it helps to append ".in":
  lp = new File(commandPath + ".in");
  if (!lp.exists() || !lp.canWrite()) {
- couldNotConnect = true;
+ couldNotPush = true;
  return;
  }
  }

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

@@ -41,7 +41,7 @@ public JabRefIcon getApplicationIcon() {
 
  @Override
  public void pushEntries(BibDatabaseContext database, List<BibEntry> entries, String keyString) {
- couldNotConnect = false;
+ couldNotPush = false;
  couldNotCall = false;
  notDefined = false;
 
@@ -74,11 +74,17 @@ public void pushEntries(BibDatabaseContext database, List<BibEntry> entries, Str
 
  @Override
  protected String[] getCommandLine(String keyString) {
+ String citeCommand = getCitePrefix();
+ // we need to escape the extra slashses
+ if (getCitePrefix().contains("\\")) {
+ citeCommand = "\"\\" + getCitePrefix();
+ }
+
  if (OS.WINDOWS) {
  // TODO we might need to escape the inner double quotes with """ """
- return new String[] {"cmd.exe", "/c", "\"" + commandPath + "\"" + "--command \"insert {\\\"characters\\\": \"\\" + getCiteCommand() + "{" + keyString + "}\"}\""};
+ return new String[] {"cmd.exe", "/c", "\"" + commandPath + "\"" + "--command \"insert {\\\"characters\\\": \"\\" + getCitePrefix() + keyString + getCiteSuffix() + "\"}\""};
  } else {
- return new String[] {"sh", "-c", "\"" + commandPath + "\"" + " --command 'insert {\"characters\": \"\\" + getCiteCommand() + "{" + keyString + "}\"}'"};
+ return new String[] {"sh", "-c", "\"" + commandPath + "\"" + " --command 'insert {\"characters\": \"" + citeCommand + keyString + getCiteSuffix() + "\"}'"};
  }
  }
 }

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

@@ -25,6 +25,6 @@ public JabRefIcon getApplicationIcon() {
 
  @Override
  protected String[] getCommandLine(String keyString) {
- return new String[] {commandPath, "--insert-cite", String.format("%s{%s}", getCiteCommand(), keyString)};
+ return new String[] {commandPath, "--insert-cite", String.format("%s%s%s", getCitePrefix(), keyString, getCiteSuffix())};
  }
 }

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

@@ -28,6 +28,6 @@ public JabRefIcon getApplicationIcon() {
 
  @Override
  protected String[] getCommandLine(String keyString) {
- return new String[] {commandPath, "-insert", getCiteCommand() + "{" + keyString + "}"};
+ return new String[] {commandPath, "-insert", getCitePrefix() + keyString + getCiteSuffix()};
  }
 }