Extract native functions to OS package

[stefan-kolb]

• Change in CHANGELOG.md described?
• Changes in pull request outlined? (What, why, ...)
• Tests created for changes?
• Tests green?

[simonharrer]

At the moment, this cannot be moved to the logic package, as it uses preferences @tobiasdiez . Maybe we should pass this setting as well as to this method/class?

[Siedlerchr]

A next step would be to integrate the code for the OpenOffice Path detection, too:
https://github.com/JabRef/jabref/blob/master/src/main/java/net/sf/jabref/openoffice/AutoDetectPaths.java

[stefan-kolb]

@Siedlerchr Good point, but this requires more work as the logic is mixed with file chooser dialogs etc.

[simonharrer]

👍

[tobiasdiez]

Nice improvement!

Does it make sense to use the state pattern here? That is, the JabrefDesktop class has an internal OS object which is initialized to the correct OS in the constructor and most of the methods just pass through to the OS class. Has the advantage, that the if(OS.Windows) ... elseif(OS.Linux) disappear.

[stefan-kolb]

@tobiasdiez Definitely worth a shot! 👍

[stefan-kolb]

The problem is that we need to have an implementation for each of the OS then + a OS-independent implementation. This is not the case which makes it hard to implement that correctly now, as far as I can see.

[tobiasdiez]

Mhh, but isn't the JabRefDesktop the OS-independent version (at least from the outside it doesn't matter what OS is used) and the newly created classes are the OS-dependent version?

My idea was to write

if(OS.Windows)
   NativeDesktop os = new Windows();
if(OS.Linux)
   NativeDesktop os = new Linux();

in the constructor of JabRefDesktop (where os is a private field) and then call for example os.openFile(link, "ps"); instead of the three if clauses.

[stefan-kolb]

Yeah, I got your idea, but there need to be a lot of things touched to make this work consistently. Let me give it a try...

[stefan-kolb]

I'll for example get problems with this method:

    public static void openFolderAndSelectFile(String fileLink) throws IOException {
        if (OS.WINDOWS) {
            WINDOWS.openFolderAndSelectFile(fileLink);
        } else if (OS.LINUX) {
            LINUX.openFolderAndSelectFile(fileLink);
        } else {
            File f = new File(fileLink);
            Desktop.getDesktop().open(f.getParentFile());
        }
    }

As openFolderAndSelect is not implemented for OSX yet.

        if(NATIVE_DESKTOP.isPresent()) {
            NATIVE_DESKTOP.get().openFolderAndSelectFile(fileLink);
        } else {
            File f = new File(fileLink);
            Desktop.getDesktop().open(f.getParentFile());
        }

[tobiasdiez]

Ah ok. If it is two much work, then just leave it as it is right now. I think it is already a big improvement over the older code.

[stefan-kolb]

Ok, I tried to unify this stuff as much as possible. Might introduce some new behavior that need to be fixed later on. Still got problems with the preferences that are used via ExternalFileType that force me to keep the code inside the gui package and JabRefDesktop for now.