-
Notifications
You must be signed in to change notification settings - Fork 1
Improved search #9
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
Conversation
petebankhead
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this better. I'm still having some trouble: for example, if I search for ROI then the ROI interface I expect doesn't appear anywhere.
This is partly because AutoCompleteTextFieldEntry.MAX_ENTRIES is quite low. Is there any reason this can't be increased, at least to 100?
Something that could help more is to specify a comparator based on the text input. For example, this should cause exact matches to bubble up to the top - and then preferentially shows matches that start with the same text. This still means that ImageJ's Roi comes on top when I search ROI (interfaces are lower), but that's still preferable to EllipseRoi.
private void setUpListeners() {
textProperty().addListener((p, o, n) -> {
String enteredText = getText();
if (enteredText == null || enteredText.isEmpty()) {
entriesPopup.hide();
} else {
String loweredCaseEnteredText = enteredText.toLowerCase();
Comparator<AutoCompleteTextFieldEntry> comparator =
Comparator.comparing((AutoCompleteTextFieldEntry e) -> e.getSearchableText().toLowerCase().equals(loweredCaseEnteredText) ? -1 : 1)
.thenComparing(e -> e.getSearchableText().toLowerCase().startsWith(loweredCaseEnteredText) ? -1 : 1)
.thenComparing(AutoCompleteTextFieldEntry::compareTo);
populatePopup(
suggestions.stream()
.filter(entry -> entry.getSearchableText().toLowerCase().contains(loweredCaseEnteredText))
.sorted(comparator)
.limit(MAX_ENTRIES)
.toList(),
enteredText
);
}
});
focusedProperty().addListener((p, o, n) -> entriesPopup.hide());
}Apache Commons Text has similarity and distance functions that might be useful, but I'm not sure - I think a few simple adjustments to the Comparator like this could be enough.
A few other things:
- Constructors include class names repeated. If I search for
ImageData, I seeImageData.ImageData(ImageServer<T>).- This example also shows a problem with my
Comparatorpreferring exact equals... typingImageDatshows me the classImageDatafirst, but typingImageDataexactly shows me the constructor first.
- This example also shows a problem with my
- I find it unexpected that selecting an item that I searched for results in the search text field showing the full item, not what I had previously typed
alanocallaghan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't build at the moment because of maven 503s but it looks good
Co-authored-by: Alan O'Callaghan <alan.ocallaghan@outlook.com>
|
All comments have been addressed |
petebankhead
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, approved now - from my side this can be merged.
Improve the Javadoc search. I made a few choices that can be discussed:
some.package.Class. The filter only looks atClass.some.package.Class.EnumorClass.Enum.variable. The filter looks atClass.EnumorEnum.variable.Class.Class(Parameter)for constructors orClass.function(Parameter)for functions. The filter looks atClassorfunction.I also did a bit of refactoring to improve error handling.