Fix opening pdf with okular in linux (#5253) #5855
Conversation
t-morales
commented
Jan 20, 2020
t-morales
commented
- Change in CHANGELOG.md described (if applicable)
- Manually tested changed features in running JabRef (always required)
- Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.
|
Thanks for your contribution, @t-morales ! Just because I'm curious: How does the fix work? Edit: nevermind, just saw the extensive discussion. ;-) |
There was a problem hiding this comment.
Ok, from my understanding it looks like your waiting for Okular to create a message in the errorstream, which you collect in the line variable. But the contents of the line variable are left unused.
Would be nice, if you could print them out in the LOGGER.
Don't be afraid of java, although it's maybe hard to master as an art, it is easy to learn and easy to start with.
There was a problem hiding this comment.
Thanks for the fix!
I did a bit of research and found the following discussion: https://stackoverflow.com/questions/10981969/why-is-going-through-geterrorstream-necessary-to-run-a-process. So apparently the problem is that the stream is full at some point which then blocks the execution of the program. It would be nice if you could add a short comment in the code explaining this and referencing the stackoverflow thread (otherwise we don't remember it in a couple of years and simply remove the code which looks unnecessary).
| p = Runtime.getRuntime().exec(cmdArray); | ||
| BufferedReader in = new BufferedReader(new InputStreamReader(p.getErrorStream())); | ||
| String line; | ||
| line = in.readLine(); |
There was a problem hiding this comment.
As @calixtus said, it would be nice if you could add a logging statement here:
Add private static final Logger LOGGER = LoggerFactory.getLogger(Linux.class); at the top of the class and then here LOGGER.debug("Received output: " + line).
There was a problem hiding this comment.
OK. I have done the modifications. I had to add also the lines
import org.slf4j.Logger; import org.slf4j.LoggerFactory;
But I am not sure how I shall proceed now? How do I modify the fix I sent? A new pull request?
Thank your for your help
There was a problem hiding this comment.
You just commit the changes on your branch and then push them to your fork.
the PR will be updated automatically
There was a problem hiding this comment.
OK. That is done. Thank you for your help!
| @@ -30,7 +30,11 @@ public void openFile(String filePath, String fileType) throws IOException { | |||
| viewer = "xdg-open"; | |||
| } | |||
| String[] cmdArray = { viewer, filePath }; | |||
| Runtime.getRuntime().exec(cmdArray); | |||
| Process p; | |||
There was a problem hiding this comment.
In java you don't need to declare and assign variables separately. So the following works (and is slightly preferred):
| Process p; | |
| Process p = Runtime.getRuntime().exec(cmdArray); |
|
Thanks for the follow-up! |
* upstream/master: Bump unirest-java from 3.4.00 to 3.4.01 (#5874) Bump junit-vintage-engine from 5.5.2 to 5.6.0 (#5875) Bump checkstyle from 8.28 to 8.29 (#5876) Bump junit-jupiter from 5.5.2 to 5.6.0 (#5877) Bump junit-platform-launcher from 1.5.2 to 1.6.0 (#5878) Change \ to / Bump byte-buddy-parent from 1.10.6 to 1.10.7 (#5873) Fix opening pdf with okular in linux (#5253) (#5855)
* master: (297 commits) Replace link to Workspace set-up with new one (#5896) Fixes making paths of linked files relative (web urls will not be touched anymore) (#5879) Switch to our IntelliJ config (#5881) Bump unirest-java from 3.4.00 to 3.4.01 (#5874) Bump junit-vintage-engine from 5.5.2 to 5.6.0 (#5875) Bump checkstyle from 8.28 to 8.29 (#5876) Bump junit-jupiter from 5.5.2 to 5.6.0 (#5877) Bump junit-platform-launcher from 1.5.2 to 1.6.0 (#5878) Change \ to / Bump byte-buddy-parent from 1.10.6 to 1.10.7 (#5873) Fix opening pdf with okular in linux (#5253) (#5855) Fixed Test Refactored constructors, PreferencesService and some minor improvements. Remove ampersand escape when writing to bib file (#5869) Fix #5862. It was indeed the throttler (at least it is working now for me) (#5868) duplicate query parameter removed (#5865) New Crowdin translations (#5864) Minor refactoring, and changed comment Bump antlr4 from 4.7.2 to 4.8-1 (#5852) Reintroducing master table index column (#5844) ...
