Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Open console" with custom command seems to pass %DIR argument wrongly #4802

Closed
geekoverdose opened this issue Mar 23, 2019 · 5 comments
Closed
Labels
bug Confirmed bugs or reports that are very likely to be bugs external files

Comments

@geekoverdose
Copy link

geekoverdose commented Mar 23, 2019

JabRef 5.0-dev, JabRef--master--latest.jar from 2019.03.23

System details: Arch Linux, i3 window manager, no desktop environment. Java version:

java version "1.8.0_201"
Java(TM) SE Runtime Environment (build 1.8.0_201-b09)
Java HotSpot(TM) 64-Bit Server VM (build 25.201-b09, mixed mode)

When using a custom command for "open terminal" the %DIR argument seems to be passed wrongly to the executed command. I've tested this with the following two configurations for the "Execute command" field for "Open console" in the settings, with details stated below:

A) "terminator --working-directory=%DIR"
B) "thunar %DIR"

A) is fault tolerant, hence also opens if the argument passed after the option is invalid. As a result, the terminal window is opened in the home directory, which is default with Terminator on my system. The interesting thing is that JabRef logs the correct commands to the logs:

INFO  org.jabref.gui.desktop.JabRefDesktop - Executing command "terminator --working-directory=/correct/path/to/library/directory"

When executing this command that JabRef logs outside JabRef in another terminal it correctly opens Terminator at the specified location.

B) is purely for debugging, as Thunar is not fault tolerant here. Thunar reports this error in a UI popup:

Error when getting information for file "/path/to/jabref-master-latest-directory/
%DIR": No such file or directory.

"/path/to/jabref-master-latest-directory/" in the directory where the nightly build of JabRef (JabRef--master--latest.jar) is located. The funny detail is that JabRef logs the correct command here too:

INFO  org.jabref.gui.desktop.JabRefDesktop - Executing command "thunar /correct/path/to/library/directory"...

Hence, the log of JabRef differs from what Thunar seems to get as an argument from JabRef. If the logged command is executed outside JabRef. it again correctly opens Thunar at the specified location. Seems that something causes the log to be different from what actually is passed to the specified external program.

Steps to reproduce the behavior:

  1. In case the bug is Linux related: use JabRef on a Linux system.
  2. Configure the command to be used for "open terminal" to one of options A) or B) above and have the corresponding software installed (Terminator, Thunar) on your system.
  3. In any opened library in JabRef execute "open terminal" --> logs correct command, seems to execute different command.
@deepakkumar96
Copy link
Contributor

Okay So, I looked into "open console" code so the issue here is:

  1. While logging command, the code does replace %DIR with the actual path,
  2. But while actually executing the command, the code does not replace %DIR with the actual path.
    (so the command is executed as it is, provided by the use)

As you can see this in code below:

if (!command.isEmpty()) {
                command = command.replaceAll("\\s+", " "); // normalize white spaces
                String[] subcommands = command.split(" ");

                // replace the placeholder if used
                String commandLoggingText = command.replace("%DIR", absolutePath);

                JabRefGUI.getMainFrame().output(Localization.lang("Executing command \"%0\"...", commandLoggingText));
                LOGGER.info("Executing command \"" + commandLoggingText + "\"...");

                try {
                   new ProcessBuilder(subcommands).start();
                } catch (IOException exception) {

Here variable 'commandLoggingText' is getting used only for logging purpose not for executing.
while this variable 'subcommand' (which is getting executed) still contains %DIR instead of the actual path.

new ProcessBuilder(subcommands).start();

@deepakkumar96
Copy link
Contributor

I'll work on this issue whenever I get time.

@geekoverdose
Copy link
Author

@deepakkumar96 Thanks, the code explains the issue well. My suggestion would be to do the %DIR replacement directly after whitespace normalization, before anything is "split" towards logging or execution.

deepakkumar96 added a commit to deepakkumar96/jabref that referenced this issue Mar 25, 2019
@tobiasdiez tobiasdiez added bug Confirmed bugs or reports that are very likely to be bugs external files labels Mar 25, 2019
Siedlerchr pushed a commit that referenced this issue Apr 13, 2019
…n the NullPointer exception (#4797)

* Add an option in preference settings to set what action to be taken by JabRef when user clicks 'open folder' on a entry.

* Fix #4763 and add added required changes in CHANGELOG.md and JabRef_en.properties

* Refactor and Reformat code.

* Fix issue #4802, where option 'open terminal here' with custom command was passing wrong argument

* Fix failure of LocalizationConsistencyTest after code changes.
@Siedlerchr
Copy link
Member

Fixed by 3d770b8

@geekoverdose
Copy link
Author

I can confirm, this successfully solved the issue. Good work again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs or reports that are very likely to be bugs external files
Projects
Archived in project
Development

No branches or pull requests

4 participants