eclipse / chemclipse

ChemClipse Project
Eclipse Public License 1.0
33 stars 18 forks source link

Remove the file dialog workaround on Windows #1761

Closed Mailaender closed 1 month ago

Mailaender commented 1 month ago

as it was silently fixed in https://github.com/eclipse-platform/eclipse.platform.swt/pull/556.

eselmeister commented 1 month ago

Why do you make it so complex and not just remove the additional handling for Windows in ExtendedFileDialog? This would have been a change of only a few lines.

Mailaender commented 1 month ago

ExtendedFileDialog is just a wrapper for the workaround.

eselmeister commented 1 month ago

Removing just the code:

if(OperatingSystemUtils.isWindows()) {
    WindowsFileDialog.clearInitialDirectoryWorkaround();
}
//

would have produced the same result. Additionally, it enables us to react quickly, if future versions of the FileHandler make problems again.

Mailaender commented 1 month ago

It is pointless to keep a function without a purpose. You always want to kill legacy code.

eselmeister commented 1 month ago

That's not legacy code. It was a convenient way to circumvent the problems on Windows we had the File Explorer. It doesn't make sense to change 100 classes if a simple change is doing also the trick. Additionally, if problems occur again, we might have to do the same work again.

Mailaender commented 1 month ago

In case you need this wrapper back, you can always revert my commit.

eselmeister commented 1 month ago

OK, I understand the reason, why you'd like to change this. It's to remove the win32 dependencies:

import org.eclipse.swt.internal.ole.win32.COM;
import org.eclipse.swt.internal.ole.win32.IFileDialog;
import org.eclipse.swt.internal.win32.OS;