Add a "ShortcutReader" interface

[madoar]

This PR adds a ShortcutReader interface for type safety reasons

[madoar]

I'm still thinking whether it is better to add the functionality of this method to the constructor.

Changing the constructor to have the signature:

public ShortcutReader(ShortcutDTO shortcut)

has the benefit that the object usage is cleaner. In addition it has the disadvantage that we can't add the constructor requirement to the interface.

[plata]

If it is part of the constructor, it cannot be defined in the interface, right?

[madoar]

Exactly

[madoar]

I'm also thinking about whether it makes sense to move the WineShortcutReader implementation to a new file.

This leads me to a second question, why do we have both a WineShortcutReader and a ShortcutReader class with the same method signatures? In theory we could call the WineShortcutReader directly if we know that the shortcut is for a wine container.

[plata]

I can't really answer this because this functionality has been there before I joined. So this is a question for @qparis. However, I would agree that implementing the ShortcutReader for each engine should be sufficient.

[madoar]

I have just created #2033, PhoenicisOrg/scripts#1068 and PhoenicisOrg/scripts#1069 to continue the open discussions.

I think we can merge this as is, and work on the created issues separately.

[madoar]

Any news on this? It would be nice if we could merge this before #2050

[plata]

This code is the same everywhere. Can we extract it to a separate method?

[madoar]

My idea was to do this later on in a centralized method in PhoenicisScriptEngine:

public interface PhoenicisScriptEngine {
    <E> E evalAndReturn(String script, Class<E> type);
    
    <E> void eval(String script, Class<E> type, Consumer<E> onSuccess, Consumer<Exception> onError);
}

See also our documentation.

For now I would suggest to retain the redundancy and fix this later for all script types (i.e. engines, installers, plugins, shortcuts, verbs etc.) at once.

[plata]

I'm fine with that.

[plata]

You need to resolve some conflicts.

[madoar]

@plata the conflicts are resolved. I would like to merge this soon because this PR blocks other useful changes to the script engine.