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

SWT GUI does not work with modern SWT library #3

Closed
knghtbrd opened this issue Nov 7, 2017 · 11 comments
Closed

SWT GUI does not work with modern SWT library #3

knghtbrd opened this issue Nov 7, 2017 · 11 comments

Comments

@knghtbrd
Copy link
Contributor

knghtbrd commented Nov 7, 2017

The Eclipse SWT libraries currently available with Linux distributions (and those that can work with any currently support JDK for macOS) do not work with AppleCommander. I never learned SWT, so I cannot easily figure out why not. It might be possible to provide a version of SWT that works on Linux, but for macOS that version just crashes when you try to run it unless the whole thing is set up to run under Apple's abandoned JRE (which has security implications.)

Apple has no JDK for that version anymore, and Oracle JDKs will not compile code that runs under Apple's (nor any) JRE that old anymore. Thanks Oracle!

It was suggested that porting the whole thing to Swing might be easier than porting it to modern SWT, but presumably it was written in SWT because Rob didn't like Swing much. (Or possibly Swing's performance which at least has been corrected.) I wasn't overly fond of Swing either, but found it a little less frustrating than AWT at least, way back when in the 1.2 daze when I used Java for anything.

Hoped the contribution of an issue might at least give us a place to discuss what might be done with/about the problem—I haven't got any easy answer here. My linux branch actually just adds a property that disables building executableGuiJar and by extension macBundle entirely when disabled. I haven't made that the default case, but it is turned on in my ACBuild.properties for my Linux branch (which exposed another issue with executableCmdJar I ought to tend to separately—it requires SWT to build because of how the compile rule is written.)

@knghtbrd
Copy link
Contributor Author

Poking around to see why I cannot select any files using the SWT GUI on macOS, I started it up by hand so that I'd have access to console messages/warnings/errors. I clicked the Open button and got this:

objc[59900]: Class FIFinderSyncExtensionHost is implemented in both /System/Library/PrivateFrameworks/FinderKit.framework/Versions/A/FinderKit (0x7fff949dda70) and /System/Library/PrivateFrameworks/FileProvider.framework/OverrideBundles/FinderSyncCollaborationFileProviderOverride.bundle/Contents/MacOS/FinderSyncCollaborationFileProviderOverride (0x141216cd8). One of the two will be used. Which one is undefined.

That may be the cause of the SWT GUI not allowing me to select a file unless I tell it to let me select all files? Can't say. I can say that once I tell it to let me select all files and pick one, an exception is being generated in com.webcodepro.applecommander.storage.filters.GraphicsFileFilter.getFileExtensions that causes the file not to open. That method is:

	/**
	 * Give file extensions.
	 */
	public static String[] getFileExtensions() {
		return referenceImage.getAvailableExtensions();
	}

referenceImage is null and there's no protection against that. Haven't figured out WHY it is null yet. I don't really have a java debugger here that works outside of an IDE, and IDEs aren't my thing, so I'm using a lot of System.out.println to dig into this so far. :)

@a2geek
Copy link
Contributor

a2geek commented Nov 16, 2017

Hey @iKarith - I just pushed an alternative build with Gradle. See the notes at DEVELOPER.md in the gradle branch. I did find that on the Mac that -XstartOnFirstThread needs to be used (per SWT documentation). Note that the builds are currently geared to Mac.

First Gradle anything, so not certain yet how to build the Win/Linux builds yet.

Also note that this generates a single JAR with all required dependencies for an easier deployment.

Some light testing seems to indicate this does function, so looking forward to your feedback as you were seeing issues.

@knghtbrd
Copy link
Contributor Author

In the meantime, I've started learning the differences between Swing (which I remember some of unfortunately) and JavaFX. One thing that is going to be obnoxious is that SWT requires -XstartOnFirstThread, but Swing and JavaFX require that you don't use that because of how THEY handle being on the right thread to do GUI stuff. sigh

The Swing app never really got done, but JavaFX doesn't seem that clumsy and clunky as Swing/AWT hybrid stuff is, so that might be the right place to go with it. I'd like to get a little bit further with it before I try to do anything with gradle, but like I said I'm open to using an alternative. If you've got it building on a Mac, especially if it builds a Mac bundle, I think Windows and Linux should be trivial. Windows tends to be the thing Java people make Just Work. :P

@knghtbrd
Copy link
Contributor Author

Disk.java has a useful convenience class to build the file selection filters for SWT, but this mechanism doesn't quite work for JavaFX. My proposed solution is possibly something we could improve upon:

diff --git a/src/com/webcodepro/applecommander/storage/Disk.java b/src/com/webcodepro/applecommander/storage/Disk.java
index dfc5f17..e3eeada 100644
--- a/src/com/webcodepro/applecommander/storage/Disk.java
+++ b/src/com/webcodepro/applecommander/storage/Disk.java
@@ -35,12 +35,15 @@ public class Disk {
 	 */
 	public class FilenameFilter {
 		private String names;
-		private String extensions;
-		public FilenameFilter(String names, String extensions) {
+		private String[] extensions;
+		public FilenameFilter(String names, String... extensions) {
 			this.names = names;
 			this.extensions = extensions;
 		}
 		public String getExtensions() {
+			return String.join(";", extensions);
+		}
+		public String[] getExtensionList() {
 			return extensions;
 		}
 		public String getNames() {
@@ -127,38 +130,38 @@ public class Disk {
 	private Disk() {
 		filenameFilters = new FilenameFilter[] {
 			new FilenameFilter(textBundle.get("Disk.AllImages"),  //$NON-NLS-1$
-				"*.do; *.dsk; *.po; *.nib; *.2mg; *.2img; *.hdv; *.do.gz; *.dsk.gz; *.po.gz; *.nib.gz; *.2mg.gz; *.2img.gz"), //$NON-NLS-1$
+				"*.do", "*.dsk", "*.po", "*.nib", "*.2mg", "*.2img", "*.hdv", "*.do.gz", "*.dsk.gz", "*.po.gz", "*.nib.gz", "*.2mg.gz", "*.2img.gz"), //$NON-NLS-1$
 			new FilenameFilter(textBundle.get("Disk.140kDosImages"),  //$NON-NLS-1$
-				"*.do; *.dsk; *.do.gz; *.dsk.gz"), //$NON-NLS-1$
+				"*.do", "*.dsk", "*.do.gz", "*.dsk.gz"), //$NON-NLS-1$
 			new FilenameFilter(textBundle.get("Disk.140kNibbleImages"), //$NON-NLS-1$
-				"*.nib; *.nib.gz"), //$NON-NLS-1$
+				"*.nib", "*.nib.gz"), //$NON-NLS-1$
 			new FilenameFilter(textBundle.get("Disk.140kProdosImages"),  //$NON-NLS-1$
-				"*.po; *.po.gz"), //$NON-NLS-1$
+				"*.po", "*.po.gz"), //$NON-NLS-1$
 			new FilenameFilter(textBundle.get("Disk.800kProdosImages"),  //$NON-NLS-1$
-				"*.2mg; *.2img; *.2mg.gz, *.2img.gz"), //$NON-NLS-1$
+				"*.2mg", "*.2img", "*.2mg.gz", "*.2img.gz"), //$NON-NLS-1$
 			new FilenameFilter(textBundle.get("Disk.ApplePcImages"),  //$NON-NLS-1$
 				"*.hdv"), //$NON-NLS-1$
 			new FilenameFilter(textBundle.get("Disk.CompressedImages"),  //$NON-NLS-1$
-				"*.sdk; *.shk; *.do.gz; *.dsk.gz; *.po.gz; *.2mg.gz; *.2img.gz"), //$NON-NLS-1$
+				"*.sdk", "*.shk", "*.do.gz", "*.dsk.gz", "*.po.gz", "*.2mg.gz", "*.2img.gz"), //$NON-NLS-1$
 			new FilenameFilter(textBundle.get("Disk.AllFiles"),  //$NON-NLS-1$
 				"*.*") //$NON-NLS-1$
 		};
 		allFileExtensions = new String[] {
 				".do",		//$NON-NLS-1$
 			    ".dsk",		//$NON-NLS-1$
 			    ".po",		//$NON-NLS-1$
 		};
 	}

Additionally, my use of some JavaFX 2.0 features and the occasional lambda where recommended in the documentation for JavaFX 2.0 are going to make this require JDK 1.8 or JDK 9 to compile. The platforms upon which that tends to be a big concern (Mac/Linux) are likely the ones that older JDKs and libraries aren't likely to work anymore anyway, I figure.

If 1.7 support is still desired, there are systems I could install that on and try to get it to work. But a big argument for JavaFX is that it's the "replacement" for Swing and it's included with 1.8+.

@knghtbrd
Copy link
Contributor Author

As I noted in #11, the issue I'm having with the SWT open dialog with FileFilter persists on Linux. At this point, resolving that closes this issue. Everything else seems to be working at this point.

@a2geek
Copy link
Contributor

a2geek commented Nov 20, 2017

I wonder if what I saw is the same thing. The "All Emulator Images" didn't actually seem to work. I picked "All Files" to see the *.dsk images in a directory. Definitely a glitch of some sort!

@knghtbrd
Copy link
Contributor Author

knghtbrd commented Nov 20, 2017

It's definitely the same issue and according to the SWT docs we're doing it right. What we're feeding it is indeed *.ext1;*.ext2;*.ext3. There were spaces originally but those aren't the problem as of my extensionList patch.

@a2geek
Copy link
Contributor

a2geek commented Nov 20, 2017

Interestingly, it does appear that *.do is functional even though *.dsk is not!

@a2geek
Copy link
Contributor

a2geek commented Nov 20, 2017

Oh! One of the files has a , instead of ; for the separator. That seems to fix the issue. Does that work for your copy?

@knghtbrd
Copy link
Contributor Author

It doesn't—even my patch above which adds getExtensionList for javafx (which works BTW) has the same problem for SWT, and it guarantees there will be no errors like that because the extensions are kept as a list of strings and String.join()ed on demand for SWT.

It doesn't work for me on Mac or on Linux. It might work on Windows—Microsoft uses the exact same mechanism so SWT is probably modeled upon that.

@knghtbrd
Copy link
Contributor Author

This issue seems fundamentally resolved. There's only two things keeping it open at this point. One of them is the yet-unmerged patch in the above comment needed for javafx (and already committed in the javafx branch anyway) and the open dialog issue which has now been separately filed as #13.

I'm closing this as resolved.

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

No branches or pull requests

2 participants