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

[NETBEANS-6329] Tools > Show In Finder don't works on JDK 17 #3866

Merged
merged 1 commit into from
Mar 27, 2022

Conversation

dyorgio
Copy link
Contributor

@dyorgio dyorgio commented Mar 25, 2022

Include exports to can execute macOs 'Show In Finder' action in jdk17.

@matthiasblaesing
Copy link
Contributor

Looks sane to me. The affected class is com.apple.eio.FileManager called from org.netbeans.modules.applemenu.ShowInFinder. It would be nice though to investigate if using java.awt.Desktop would be an option (the browse method in that class does the right thing (at least looks like it) on linux.

@matthiasblaesing matthiasblaesing added this to the NB14 milestone Mar 25, 2022
@dyorgio
Copy link
Contributor Author

dyorgio commented Mar 25, 2022

Hi @matthiasblaesing, yes it does, but only if you call getParentFile:

// Current Implementation
final Class<?> fmClz = Class.forName("com.apple.eio.FileManager");      //NOI18N
final Method m = fmClz.getDeclaredMethod("revealInFinder", File.class); //NOI18N
m.invoke(null, new File("/PATH/FILE.EXT"));

// Suggested Implementation
Desktop.getDesktop().browse(new File("/PATH/FILE.EXT").getParentFile().toURI());

But I only tested on macOS 12.3 (latest), with 1.8 and 17 java versions.

@matthiasblaesing
Copy link
Contributor

Unittests are green - @dyorgio I suggest to merge this as is. It is low risk and fixes a problem, so it is an improvement in anycase.

Independent of that it would be good to get rid of the add-exports if possible, so if you would follow up with the suggested implementation.

Please indicate if you agree that I merge now.

@dyorgio
Copy link
Contributor Author

dyorgio commented Mar 26, 2022

Yes I also prefer the original solution.
We cannot guarantee that another versions of macOS will work with Desktop.browser, I suspect that it is the reason to use platform specific implementation.

@matthiasblaesing matthiasblaesing merged commit 3fea7d8 into apache:master Mar 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants