DefaultModuleService: change persisting of items

[stelfrich]

Disable the persisting of a ModuleItems if an initialize() method is set and is not empty. This way, initialize() can overwrite the value of a ModuleItem even if persist=true is assumed implicitly.

See:

• https://github.com/imagej/imagej-legacy/pull/137#issuecomment-200543860

[ctrueden]

Did this work well in your tests?

[stelfrich]

It works as I expect it to work after resetting the Prefs. That leaves us with two issues:

1. If a ModuleItem has been persisted previously, this value will still overwrite initialize()
2. With this change, explicitly setting persist=true for a ModuleItem that also has initializer set, will disable persisting (that's what we have expected)

I couldn't find any other odd behavior. But as you have already mentioned, we might be breaking someone else's code with this change.

[stelfrich]

With this change, explicitly setting persist=true for a ModuleItem that also has initializer set, will disable persisting (that's what we have expected)

I have added clarifying documentation in Parameter.

[stelfrich]

Thanks for the comments (and being so patient), @ctrueden! They have been addressed.

[ctrueden]

I gave this a little more thought, and here is what I would like to do:

• Change persist to a ternary enum: YES, NO and DEFAULT or some such. We can have DEFAULT behave as you have coded here—and in fact, tweak the persistence heuristic as needed for what we deem is most reasonable going forward. If someone says YES, they know better, and want it saved. And similarly, NO means NO. :wink:

I would like to include this update as part of the SJC3 release, due within the next few months. There is an sjc3 integration branch containing all such changes, where the above changes should go.

If you have time to tackle it, great. Or if not, I will take care of it before SJC3 is released.

[stelfrich]

If you have time to tackle it, great.

I did, @ctrueden. But that invalidates all Commands with bytecode of the previous Parameter implementation, right? How would I test this implementation in an ImageJ2 installation - I am a bit lost here :confused:

[ctrueden]

FYI, my plan is to merge this into the sjc3 branch. Will do soon.

[ctrueden]

Just a quick update: I looked into doing this earlier in January, but the sjc3 branch of scijava-common is quite out of date. It needs a rebase over the latest scijava-common. In particular, all the logging updates are now incompatible with the changes on master as well as the log-revamp branch.

So it will be a while longer before I have time to get all this unified. Probably April. I am anxious to get SJC3 finished and merged, but it's going to be a substantial endeavor.

The good news is that I won't forget about this PR, thanks to its inclusion in the 3.0.0 milestone.