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

Desktop: Add option to select the search result order #7622

Closed
wants to merge 24 commits into from

Conversation

JackGruber
Copy link
Contributor

Add option to select the search result order.
grafik

The topic of search result order has already come up several times and the relevance (bm25) sorting is not always what the user wants.

https://discourse.joplinapp.org/t/search-result-order/3763/12
https://discourse.joplinapp.org/t/sort-search-results-by-date/28875
#1388
#5524
#4220

Changes made

  • Add a menue option to sort search results by BM25, title, creation or updated date
  • BM25 is selected as the default sort order
  • For title sorting the localeCompare is used to get numbers in strings correct sorted
  • SearchEngine.test.js converted into typescript

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@JackGruber
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@Daeraxa
Copy link
Collaborator

Daeraxa commented Jan 15, 2023

I think maybe the bm25 part of the label should be left off, I feel that most users aren't going to know what it means and might not understand that it was the default one so will avoid choosing it.

@chevdor
Copy link
Sponsor

chevdor commented Jan 15, 2023

I def. welcome this PR, thanks for the great work @JackGruber.

My only question is whether we really need to have (and potentially confuse the user) an option for the note list and another for the search. If it is easier to merge this PR without making such a decision yet, I think this would already provide a good value in the current state.

@JackGruber
Copy link
Contributor Author

I first thought about using the option to display the notes, but since the search also has a relevance-based sorting, the same option cannot be used.

@JackGruber
Copy link
Contributor Author

After a bit of thought, it might be possible to add a Sort search by relevance option like the Reverse sort order and only sort by relevance if this is ticked, otherwise sort like the note sort.
It's not quite as flexible, but I think it's ok, and for relevance sorting, ignore than the reverse order option.

@laurent22
Copy link
Owner

I wish this had been discussed on the forum first so as not to spend too much time on the implementation. I'm not very keen to add this to be honest because I don't think it's the right solution to whatever problem there is.

When I look at the linked threads, it seems people usually want to sort by date, but Naveen added this at some point - more recent notes have a higher weight. So maybe it's a matter of tweaking the algorithm further and give more importance to the note date.

Another advantage of improving the search engine itself is that it will benefit the mobile app, CLI, API and plugins too.

@JackGruber
Copy link
Contributor Author

JackGruber commented Jan 16, 2023

as not to spend too much time on the implementation.

This is no problem, since i have only adapted this implementation a bit and added the test, since i use it myself, since i don't like the relevancy sorting and and therefore build my own application.

If there is no interest in the PR, just close it.

@chevdor
Copy link
Sponsor

chevdor commented Jan 16, 2023

Please don't delete the PR's branch, until there is a full spec and matching implementation, I think your option is great @JackGruber, thanks for spending some time on that and sharing.

I personally don't need/want to sort by <you name it>, instead what I need is it depends !
Sometimes I need to sort by note title, sometimes by creation date, sometimes by updated date, size, etc....
So I don't think we can find the right sorting algorithm but need to let the user switch back and forth between a few options.

@laurent22
Copy link
Owner

This is no problem, since i have only adapted this implementation a bit and added the test, since i use it myself, since i don't like the relevancy sorting and and therefore build my own application.

Ok that's great if it's still of use to you. And I appreciate you spending time creating the PR but I feel what's missing, but that might be for another day, is a review of search in existing apps, weighting the pros and cons, and finding a solution that fits Joplin well. In the meantime I will close the PR then.

@laurent22 laurent22 closed this Jan 16, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants