imagej / imagej-legacy

ImageJ+ImageJ2 compatibility layer
https://imagej.net/libs/imagej-legacy
BSD 2-Clause "Simplified" License
16 stars 25 forks source link

MacroPreprocessor: resolve Button type inputs #262

Open imagejan opened 3 years ago

imagejan commented 3 years ago

When calling a command from a macro (with an option string), we usually don't want to show a UI dialog.

If the called command contains inputs of type org.scijava.widget.Button, we resolve these without setting them, as we assume that buttons and their callbacks provide some interactivity that is not required when running from macro.

See the closed PR https://github.com/imagej/imagej-legacy/pull/239 for some discussion.


I would have liked to respect a required={false,true} annotation for the Button input, but since Button extends Optional, isRequired() will always resolve to false, see also:

https://github.com/scijava/scijava-common/blob/46ca0d04c5c86eeb51f9daa9f130b0d7c8decd65/src/main/java/org/scijava/command/CommandModuleItem.java#L102-L106


I hope that this solution - while being a bit hacky - is less intrusive than #239, but still meets the requirements of the affected projects.

(/cc @frauzufall @NicoKiaru @tischi @romainGuiet @uschmidt83 @tferr)

Note 1: While in full-fledged scripting languages, you can always call commandService.run("command",false, ...) to avoid the automatic command preprocessing (and hence display of dialogs), this option doesn't exist in an IJ1 macro run("...") call.

Note 2: This pull request still allows commands whose dialog box is nothing else than a button to show a dialog box when invoked from the UI (scenario 1 in this comment by @ctrueden).

Note 3: I tried to add a test for this behavior in https://github.com/imagej/imagej-legacy/commit/401cb9c85dc53646c4f8faced35a08d42d75bf4e, but it's not headless, as it seems this issue only occurs when running from a macro with the UI displayed. In addition, we still cannot distinguish between macro calls with and without option string (run("Test Command") vs. run("Test Command", ""), see also https://github.com/scijava/scijava-common/issues/317#issuecomment-364458528).

imagejan commented 2 years ago

See https://github.com/imagej/imagej-legacy/pull/239#issuecomment-1125729713, thanks @biovoxxel for testing!

@hinerm @ctrueden is there any chance we can get this merged and released soon?