Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SwingObjectWidget: set values by string comparison #87

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

imagejan
Copy link
Member

@imagejan imagejan commented Mar 28, 2025

Fixes scijava/scijava-common#471.

Previously, we were trying to set the JComboBox value with a new instance of the object.

We don't always have the possibility to keep a list of the allowed choices for the current model, nor can we override the equals() method for the Objects of this widget instance.
So we have to fall back to comparing all combobox items to the current value by their toString() and ObjectService.getName() strings.

Previously, we were trying to set the JComboBox value
with a new instance of the object.

We don't always have the possibility to keep a list of the allowed
choices for the current model, nor can we override the `equals()` method
for the `Object`s of this widget instance.
So we have to fall back to comparing all combobox items to the current
value by their `toString()` and `ObjectService.getName()` strings.
@imagejan imagejan requested a review from ctrueden March 28, 2025 15:17
@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/scijava-persist-not-working-for-enums/99827/9

@ctrueden
Copy link
Member

ctrueden commented Apr 3, 2025

This is awesome. The reason I didn't merge it instantly is because switching to string comparison over reference and/or object equality is a fundamental change in how the widgets behave, and it honestly makes me nervous that there will be unintended consequences. E.g. it's possible to have two different Datasets or ImagePluses with the same name, which will be rendered identically in the widget, which could (according to my vague intuition, without having tested yet) result in the wrong object being selected and filled in some scenarios.

So... testing needed. I hope to get to it extremely soon; I know how annoying this bug has been.

@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/fiji-friends-weekly-dev-update-thread/103718/84

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Persistence of enum choices in Commands
3 participants