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.

Sign up for GitHub

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

Jump to bottom

fix: xdg-open is now used to reveal a file Linux file system (#674) #711

Merged
merged 34 commits into from
Oct 1, 2024
Merged

fix: xdg-open is now used to reveal a file Linux file system (#674) #711

merged 34 commits into from
Oct 1, 2024

Conversation

Oliver-Loeffler
Copy link
Collaborator

@Oliver-Loeffler Oliver-Loeffler commented Sep 4, 2024 •
edited
Loading

This PR removes the call to nautilus as file browser and makes use of xdg-open instead.
On most recent Linux OS, a call to xdg-open /home/user/myfxmls will open the directory in a file browser which is specific to the OS and desktop environment.

There is one part missing yet, the error handling. Its complained that only an IOException is reported. This is indeed quite misleading. A subsequent update of this PR will introduce proper message dialog and show whats wrong.

Issue

Fixes #674

Progress

@Oliver-Loeffler Oliver-Loeffler added the bug Something isn't working label Sep 4, 2024
@Oliver-Loeffler Oliver-Loeffler self-assigned this Sep 4, 2024
@Oliver-Loeffler Oliver-Loeffler marked this pull request as draft September 4, 2024 06:11
Oliver-Loeffler and others added 8 commits September 4, 2024 08:13
@Oliver-Loeffler
Copy link
Collaborator Author

Instead of Nautilus, xdg-open is now used to open a file system view on Linux. Also, the actual return code of the cmd calls used is now evaluated. In case of error, this is reported to the user. Furthermore, all error dialogs for related open, reveal and edit operations now look the same (short error message, Scene Builder icon, a details window with the stack trace). Logging has been added where it seemed useful.

@Oliver-Loeffler Oliver-Loeffler marked this pull request as ready for review September 24, 2024 12:59
@Oliver-Loeffler Oliver-Loeffler added this to the 24 milestone Sep 27, 2024
@Oliver-Loeffler Oliver-Loeffler changed the title fix: xdg-open is now used to reveal a file in the operating system default file browser (#674) fix: xdg-open is now used to reveal a file Linux file system (#674) Sep 29, 2024
jperedadnr
jperedadnr reviewed Sep 29, 2024
View reviewed changes
kit/src/main/java/com/oracle/javafx/scenebuilder/kit/editor/EditorPlatform.java Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
app/src/main/java/com/oracle/javafx/scenebuilder/app/DocumentWindowController.java Show resolved Hide resolved
app/src/main/java/com/oracle/javafx/scenebuilder/app/DocumentWindowController.java Show resolved Hide resolved
app/src/main/resources/com/oracle/javafx/scenebuilder/app/i18n/SceneBuilderApp.properties Show resolved Hide resolved
kit/src/main/java/com/oracle/javafx/scenebuilder/kit/editor/EditorController.java Outdated Show resolved Hide resolved
kit/src/main/java/com/oracle/javafx/scenebuilder/kit/editor/EditorPlatform.java Show resolved Hide resolved
kit/src/main/resources/com/oracle/javafx/scenebuilder/kit/i18n/SceneBuilderKit.properties Show resolved Hide resolved
kit/src/test/resources/com/oracle/javafx/scenebuilder/kit/editor/timeout.cmd Outdated Show resolved Hide resolved
kit/src/test/resources/com/oracle/javafx/scenebuilder/kit/editor/timeout.sh Outdated Show resolved Hide resolved
@ktgw0316 ktgw0316 mentioned this pull request Sep 30, 2024
Closed
@Oliver-Loeffler
Copy link
Collaborator Author

Oliver-Loeffler commented Sep 30, 2024 •
edited
Loading

@jperedadnr I will revert this so that the actual work is still performed in JavaFX application thread as before this change.
After reviewing the code and looking where possible changes have to be implemented, I thing filing another issue for refactoring is a better ider. Otherwise this PR becomes too large. It already touches quite many files.

Main purpose is to address the quality of the error messages and the use of xdg-open instead of nautilus. The issue for the anyway required refactoring is #737.

The idea to have this configurable is tracked in issue #738.

jperedadnr
jperedadnr approved these changes Sep 30, 2024
View reviewed changes
Copy link
Collaborator

@jperedadnr jperedadnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

ktgw0316 added a commit to ktgw0316/scenebuilder that referenced this pull request Oct 1, 2024
@ktgw0316
Add Japanese translation for new properties
0449910 
References:
* 13e7ce4
* gluonhq#711 (comment)
ktgw0316 added a commit to ktgw0316/scenebuilder that referenced this pull request Oct 1, 2024
@ktgw0316
Add Japanese translation for new properties
326ea55 
References:
* 13e7ce4
* gluonhq#711 (comment)
ktgw0316 added a commit to ktgw0316/scenebuilder that referenced this pull request Oct 1, 2024
@ktgw0316
Add Japanese translation for new properties
f4525ba 
References:
* 13e7ce4
* gluonhq#711 (comment)
@Oliver-Loeffler Oliver-Loeffler merged commit f0c5860 into gluonhq:master Oct 1, 2024
3 checks passed
ktgw0316 added a commit to ktgw0316/scenebuilder that referenced this pull request Oct 2, 2024
@ktgw0316
Add Japanese translation for new properties
bbdffa0 
References:
* 13e7ce4
* gluonhq#711 (comment)
@weg2022 weg2022 mentioned this pull request Oct 3, 2024
Merged
4 tasks
ktgw0316 added a commit to ktgw0316/scenebuilder that referenced this pull request Oct 3, 2024
@ktgw0316
Add Japanese translation for new properties
15c26d2 
References:
* 13e7ce4
* gluonhq#711 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jperedadnr jperedadnr jperedadnr approved these changes

@abhinayagarwal abhinayagarwal Awaiting requested review from abhinayagarwal

Assignees

@Oliver-Loeffler Oliver-Loeffler

Labels
bug Something isn't working
Projects
None yet
Milestone
24
Development

Successfully merging this pull request may close these issues.

Opening the OS-supplied File Manager (other than Nautilus) fail
2 participants
@Oliver-Loeffler @jperedadnr