Fix linux file opening by using Process Builder #8853

Siedlerchr · 2022-05-26T15:50:51Z

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Fixes #8831 Co-authored-by: ThiloteE <[email protected]>

Co-authored-by: ThiloteE <[email protected]>

Siedlerchr · 2022-05-26T17:08:52Z

Terminal opening works, but it doesn't openin the folder, althought directly executing the following command works:
`gnome-terminal --working-directory="/home/xxx/xxxxxr/00 xxxx xxx xx"

`

src/main/java/org/jabref/gui/desktop/os/Linux.java

btut

Is there a particular reason you want to change to ProcessBuilder?

src/main/java/org/jabref/gui/desktop/os/Linux.java

Co-authored-by: Siedlerchr [email protected]

btut · 2022-05-31T18:46:08Z

Sorry for not giving credit, I somehow screwed up the co-authored-by.

ThiloteE · 2022-05-31T19:01:13Z

@btut Ok, I did a short test after you did the split: Linux Mint Cinnamon 20.3 with Nemo filemanager on first glance works perfectly :-)

btut · 2022-05-31T19:03:48Z

src/main/java/org/jabref/gui/desktop/os/Linux.java

-        Process p = runtime.exec("readlink /etc/alternatives/x-terminal-emulator");
-        BufferedReader reader = new BufferedReader(new InputStreamReader(p.getInputStream()));
-
-        String emulatorName = reader.readLine();


I get null here. I think /etc/alternatives is a Debian thing.

Maybe if (Files.exists(/etc/alternatives...) ?

Yes, but what if it does not exist? What terminal emulator do we choose then?

Turns out there is no real solution, just checking multiple places https://superuser.com/a/1461079

So how do we proceed? Leave it as is? Show an alert message that we cannot determine a TE?
Do you think this is a feature that many people use?

Show an alert message. Unable to open terminal. I doubt many use this

ilippert · 2022-06-01T13:35:31Z

as per #8849 (comment): the fix works on Fedora's default Gnome, too.

* upstream/main: Add an importer for Citavi backup files (#8848) Reviewdoc: Comment on PRs (#8878) Squashed 'buildres/csl/csl-styles/' changes from 649aac4..e740261 Use JDK 15 text blocks to improve injected languages readability (#8874) Fix fetcher tests (#8877) Fix #8390 by allowing multiple group deletion for Remove groups > Kee… (#8875) Add restart warning on SSL configuration change (#8871) Update to lucene 9.2 (#8868) Fix for removing several groups deletes only one of them (#8801) Disable Write XMP Button in General tab of Entry-Editor when action is in progress (#8728) Bump jsoup from 1.14.3 to 1.15.1 (#8864) Bump unirest-java from 3.13.8 to 3.13.10 (#8869) Bump unoloader from 7.3.2 to 7.3.3 (#8863) Bump pascalgn/automerge-action from 0.15.2 to 0.15.3 (#8860) Bump classgraph from 4.8.146 to 4.8.147 (#8861) Bump mockito-core from 4.5.1 to 4.6.0 (#8862) Lucence dir checkers should only delete lucence dirs (#8854) Update README.md (#8858) Update adr.md Update adr.md

Set directory for terminal

github-actions · 2022-06-01T19:31:03Z

src/main/java/org/jabref/gui/desktop/os/Linux.java

+                    cmd = new String[] {emulatorName, absolutePath};
+                }
+
+                ProcessBuilder builder  = new ProcessBuilder(cmd);


🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.whitespace.SingleSpaceSeparatorCheck> _{reported by reviewdog 🐶}
Use a single space to separate non-whitespace characters.

github-actions · 2022-06-01T19:31:03Z

src/main/java/org/jabref/gui/libraryproperties/constants/ConstantsPropertiesViewModel.java

        this.databaseContext = databaseContext;
+        this.dialogService  = dialogService;


🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.whitespace.SingleSpaceSeparatorCheck> _{reported by reviewdog 🐶}
Use a single space to separate non-whitespace characters.

github-actions · 2022-06-01T19:31:03Z

src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneView.java

@@ -51,7 +53,7 @@ private void initialize() {
        ActionFactory factory = new ActionFactory(preferences.getKeyBindingRepository());
        EasyBind.subscribe(viewModel.selectedFetcherProperty(), fetcher -> {
            if ((fetcher != null) && fetcher.getHelpPage().isPresent()) {
-                Button helpButton = factory.createIconButton(StandardActions.HELP, new HelpAction(fetcher.getHelpPage().get()));
+                Button helpButton = factory.createIconButton(StandardActions.HELP, new HelpAction(fetcher.getHelpPage().get(),dialogService));


🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.whitespace.WhitespaceAfterCheck> _{reported by reviewdog 🐶}
',' is not followed by whitespace.

Siedlerchr · 2022-06-01T20:54:07Z

TODO: l10n Could\ not\ detect\ terminal\ automatically.\ Please\ define\ a\ custom\ terminal\ in\ the\ preferences.=Could not detect terminal automatically. Please define a custom terminal in the preferences.

calixtus · 2022-06-01T23:37:06Z

reveal in file explorer works with arch linux, kde and dolphin,
what does not work here is open terminal here with only konsole installed instead of xterm.

* upstream/main: Add ADR 25 to ADR Toc (#8882)

Siedlerchr and others added 4 commits May 26, 2022 17:50

Fix linux file opening by using Process Builder

9b2640c

Fixes #8831 Co-authored-by: ThiloteE <[email protected]>

checkstyle

11cd454

add comment on nemo

1391274

Co-authored-by: ThiloteE <[email protected]>

Refactor terminal opening as well

0ad5405

Co-authored-by: ThiloteE <[email protected]>

Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 26, 2022

ThiloteE added ui os: linux external files labels May 27, 2022

Siedlerchr requested a review from btut May 30, 2022 16:21

btut reviewed May 31, 2022

View reviewed changes

src/main/java/org/jabref/gui/desktop/os/Linux.java Outdated Show resolved Hide resolved

btut reviewed May 31, 2022

View reviewed changes

src/main/java/org/jabref/gui/desktop/os/Linux.java Outdated Show resolved Hide resolved

src/main/java/org/jabref/gui/desktop/os/Linux.java Show resolved Hide resolved

Split ProcessBuilder arguments into Array

4c21b36

Co-authored-by: Siedlerchr [email protected]

btut reviewed May 31, 2022

View reviewed changes

Siedlerchr added 2 commits June 1, 2022 20:51

Refactor passing of dialog servie

51d9b32

Set directory for terminal

github-actions bot reviewed Jun 1, 2022

View reviewed changes

Siedlerchr added 3 commits June 1, 2022 22:15

fix checkstyle

8d2527b

checkstyle

b587215

remove blank line

f607ee9

Siedlerchr added 3 commits June 5, 2022 17:18

Merge remote-tracking branch 'upstream/main' into fileManager

7475105

* upstream/main: Add ADR 25 to ADR Toc (#8882)

add missing l10n

ceca3bf

add changelog

7b263a6

Siedlerchr merged commit 35cd5d0 into main Jun 5, 2022

Siedlerchr deleted the fileManager branch June 5, 2022 16:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix linux file opening by using Process Builder #8853

Fix linux file opening by using Process Builder #8853

Siedlerchr commented May 26, 2022 •

edited by ThiloteE

Loading

Siedlerchr commented May 26, 2022

btut left a comment

btut commented May 31, 2022

ThiloteE commented May 31, 2022

btut May 31, 2022

Siedlerchr May 31, 2022

btut May 31, 2022

Siedlerchr May 31, 2022

btut Jun 1, 2022

Siedlerchr Jun 1, 2022

ilippert commented Jun 1, 2022

github-actions bot Jun 1, 2022

github-actions bot Jun 1, 2022

github-actions bot Jun 1, 2022

Siedlerchr commented Jun 1, 2022

calixtus commented Jun 1, 2022

		this.databaseContext = databaseContext;
		this.dialogService = dialogService;

Fix linux file opening by using Process Builder #8853

Fix linux file opening by using Process Builder #8853

Conversation

Siedlerchr commented May 26, 2022 • edited by ThiloteE Loading

Siedlerchr commented May 26, 2022

btut left a comment

Choose a reason for hiding this comment

btut commented May 31, 2022

ThiloteE commented May 31, 2022

btut May 31, 2022

Choose a reason for hiding this comment

Siedlerchr May 31, 2022

Choose a reason for hiding this comment

btut May 31, 2022

Choose a reason for hiding this comment

Siedlerchr May 31, 2022

Choose a reason for hiding this comment

btut Jun 1, 2022

Choose a reason for hiding this comment

Siedlerchr Jun 1, 2022

Choose a reason for hiding this comment

ilippert commented Jun 1, 2022

github-actions bot Jun 1, 2022

Choose a reason for hiding this comment

github-actions bot Jun 1, 2022

Choose a reason for hiding this comment

github-actions bot Jun 1, 2022

Choose a reason for hiding this comment

Siedlerchr commented Jun 1, 2022

calixtus commented Jun 1, 2022

Siedlerchr commented May 26, 2022 •

edited by ThiloteE

Loading