search

genericworkflownodes / GenericKnimeNodes

Base package for GenericKnimeNodes
https://github.com/genericworkflownodes/GenericKnimeNodes
Other
15 stars 16 forks source link

Input Files "resolve to workflow relative path" #216

Closed jpfeuffer closed 5 years ago

jpfeuffer commented 5 years ago

I think it would make sense to change it to: "Resolve against KNIME path" and then offer a Textbox where Users can put knime://knime.workspace knime://knime.workflow knime://knime.TheOtherThing

I saw this because I wanted to put data workspace-relative to share it among workflows.

AlexanderFillbrunn commented 5 years ago

The current implementation also has a bug. When you reset the node and execute it again, it gives the following error: Configure failed (IllegalArgumentException): 'other' is different type of Path.

Additionally the path does get resolved correctly, but the original (non-relative) path is written to the settings. How do we actually want to handle this correctly @jpfeuffer? When the user closes the window, should all the paths be converted to knime://... URIs if the corresponding setting is set? What happens if the user opens the dialog again and unchecks the box? Then we should convert the files back, right? Should that all happen automatically or would it be better to have a button "Convert URIs to ...", where "..." is "Workflow relative", "Mountpoint relative", "Specific mountpoint relative" and "Normal"?

jpfeuffer commented 5 years ago

Hm I see. Maybe the button is the better idea. Important would be, that if you send the workflow (and the saved settings, potentially resetted) together with a workspace of data, it should be executable right away without opening (since it is potentially used from the command line). So in the settings there should definitely be the resolved path stored. And to make it clearer for the user, it is maybe better to show the change right away in the dialog when clicking a "Convert" button. Then the node only saves what the user also sees. What do you think? Am I seeing that right?

AlexanderFillbrunn commented 5 years ago

Yes, that's how I also see it! I will have a look at the necessary changes.

AlexanderFillbrunn commented 5 years ago

@jpfeuffer It would probably make sense to put this optionally in the DialogComponentMultiFileChooser somehow, so it is available everywhere where this component is used, don't you think?

jpfeuffer commented 5 years ago

True, would be cleaner. It is heavily connected to this component anyway right?

AlexanderFillbrunn commented 5 years ago

Yes, currently it is only used in the Input Files node anyways, but we can make the buttons optional using another constructor, so we don't have to change anything else. When the user clicks one of the buttons, the list in the component is simply changed.

AlexanderFillbrunn commented 5 years ago

Based on some Skype discussion with @jpfeuffer I opened PR #234 that adds a button "Convert..." that allows users to convert the file paths in the list to various relative knime:// paths and back to absolute paths.