From 556c4ac358fb3b88acc9b998125da42176801758 Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Wed, 26 Apr 2023 17:23:08 +0100 Subject: [PATCH] LibGUI: Allow FilteringProxyModel to optionally sort results by score When the `FilteringOptions::SortByScore` flag is set, filtered indices are sorted by match score in descending order, meaning the most relevant results should appear first. The default behavior of FilteringProxyModel is unchanged. --- .../Applications/Browser/CookiesModel.cpp | 11 +++++---- Userland/Applications/Browser/CookiesModel.h | 2 +- .../Browser/History/HistoryModel.cpp | 11 +++++---- .../Browser/History/HistoryModel.h | 2 +- .../Applications/Browser/StorageModel.cpp | 11 +++++---- Userland/Applications/Browser/StorageModel.h | 2 +- Userland/Applications/Help/ManualModel.cpp | 15 +++++++----- Userland/Applications/Help/ManualModel.h | 2 +- Userland/Demos/ModelGallery/BasicModel.cpp | 12 ++++++---- Userland/Demos/ModelGallery/BasicModel.h | 2 +- Userland/Libraries/LibGUI/CommandPalette.cpp | 11 +++++---- .../Libraries/LibGUI/FilteringProxyModel.cpp | 23 ++++++++++++------- .../Libraries/LibGUI/FilteringProxyModel.h | 23 +++++++++++++++---- Userland/Libraries/LibGUI/ItemListModel.h | 6 ++--- Userland/Libraries/LibGUI/Model.h | 7 +++++- 15 files changed, 89 insertions(+), 51 deletions(-) diff --git a/Userland/Applications/Browser/CookiesModel.cpp b/Userland/Applications/Browser/CookiesModel.cpp index fc2c99bb64..3fbe1f8787 100644 --- a/Userland/Applications/Browser/CookiesModel.cpp +++ b/Userland/Applications/Browser/CookiesModel.cpp @@ -88,17 +88,18 @@ GUI::Variant CookiesModel::data(GUI::ModelIndex const& index, GUI::ModelRole rol VERIFY_NOT_REACHED(); } -TriState CookiesModel::data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const +GUI::Model::MatchResult CookiesModel::data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const { auto needle = term.as_string(); if (needle.is_empty()) - return TriState::True; + return { TriState::True }; auto const& cookie = m_cookies[index.row()]; auto haystack = DeprecatedString::formatted("{} {} {} {}", cookie.domain, cookie.path, cookie.name, cookie.value); - if (fuzzy_match(needle, haystack).score > 0) - return TriState::True; - return TriState::False; + auto match_result = fuzzy_match(needle, haystack); + if (match_result.score > 0) + return { TriState::True, match_result.score }; + return { TriState::False }; } Web::Cookie::Cookie CookiesModel::take_cookie(GUI::ModelIndex const& index) diff --git a/Userland/Applications/Browser/CookiesModel.h b/Userland/Applications/Browser/CookiesModel.h index 22f6a9964a..b34b0e9350 100644 --- a/Userland/Applications/Browser/CookiesModel.h +++ b/Userland/Applications/Browser/CookiesModel.h @@ -33,7 +33,7 @@ public: virtual String column_name(int column) const override; virtual GUI::ModelIndex index(int row, int column = 0, GUI::ModelIndex const& = GUI::ModelIndex()) const override; virtual GUI::Variant data(GUI::ModelIndex const& index, GUI::ModelRole role = GUI::ModelRole::Display) const override; - virtual TriState data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const override; + virtual GUI::Model::MatchResult data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const override; Web::Cookie::Cookie take_cookie(GUI::ModelIndex const&); AK::Vector take_all_cookies(); diff --git a/Userland/Applications/Browser/History/HistoryModel.cpp b/Userland/Applications/Browser/History/HistoryModel.cpp index 17b8c2f533..6fbee523e1 100644 --- a/Userland/Applications/Browser/History/HistoryModel.cpp +++ b/Userland/Applications/Browser/History/HistoryModel.cpp @@ -72,17 +72,18 @@ GUI::Variant HistoryModel::data(GUI::ModelIndex const& index, GUI::ModelRole rol VERIFY_NOT_REACHED(); } -TriState HistoryModel::data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const +GUI::Model::MatchResult HistoryModel::data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const { auto needle = term.as_string(); if (needle.is_empty()) - return TriState::True; + return { TriState::True }; auto const& history_entry = m_entries[index.row()]; auto haystack = DeprecatedString::formatted("{} {}", history_entry.title, history_entry.url.serialize()); - if (fuzzy_match(needle, haystack).score > 0) - return TriState::True; - return TriState::False; + auto match_result = fuzzy_match(needle, haystack); + if (match_result.score > 0) + return { TriState::True, match_result.score }; + return { TriState::False }; } } diff --git a/Userland/Applications/Browser/History/HistoryModel.h b/Userland/Applications/Browser/History/HistoryModel.h index dc18d8dd2e..5b89da9ee3 100644 --- a/Userland/Applications/Browser/History/HistoryModel.h +++ b/Userland/Applications/Browser/History/HistoryModel.h @@ -28,7 +28,7 @@ public: virtual String column_name(int column) const override; virtual GUI::ModelIndex index(int row, int column = 0, GUI::ModelIndex const& = GUI::ModelIndex()) const override; virtual GUI::Variant data(GUI::ModelIndex const& index, GUI::ModelRole role = GUI::ModelRole::Display) const override; - virtual TriState data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const override; + virtual GUI::Model::MatchResult data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const override; private: AK::Vector m_entries; diff --git a/Userland/Applications/Browser/StorageModel.cpp b/Userland/Applications/Browser/StorageModel.cpp index 4dc22696a0..879bee6baf 100644 --- a/Userland/Applications/Browser/StorageModel.cpp +++ b/Userland/Applications/Browser/StorageModel.cpp @@ -75,20 +75,21 @@ GUI::Variant StorageModel::data(GUI::ModelIndex const& index, GUI::ModelRole rol VERIFY_NOT_REACHED(); } -TriState StorageModel::data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const +GUI::Model::MatchResult StorageModel::data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const { auto needle = term.as_string(); if (needle.is_empty()) - return TriState::True; + return { TriState::True }; auto const& keys = m_local_storage_entries.keys(); auto const& local_storage_key = keys[index.row()]; auto const& local_storage_value = m_local_storage_entries.get(local_storage_key).value_or({}); auto haystack = DeprecatedString::formatted("{} {}", local_storage_key, local_storage_value); - if (fuzzy_match(needle, haystack).score > 0) - return TriState::True; - return TriState::False; + auto match_result = fuzzy_match(needle, haystack); + if (match_result.score > 0) + return { TriState::True, match_result.score }; + return { TriState::False }; } } diff --git a/Userland/Applications/Browser/StorageModel.h b/Userland/Applications/Browser/StorageModel.h index c1177c8738..92ed8bcf62 100644 --- a/Userland/Applications/Browser/StorageModel.h +++ b/Userland/Applications/Browser/StorageModel.h @@ -25,7 +25,7 @@ public: virtual String column_name(int column) const override; virtual GUI::ModelIndex index(int row, int column = 0, GUI::ModelIndex const& = GUI::ModelIndex()) const override; virtual GUI::Variant data(GUI::ModelIndex const& index, GUI::ModelRole role = GUI::ModelRole::Display) const override; - virtual TriState data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const override; + virtual GUI::Model::MatchResult data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const override; private: OrderedHashMap m_local_storage_entries; diff --git a/Userland/Applications/Help/ManualModel.cpp b/Userland/Applications/Help/ManualModel.cpp index bbee48ec02..b0c61a8882 100644 --- a/Userland/Applications/Help/ManualModel.cpp +++ b/Userland/Applications/Help/ManualModel.cpp @@ -208,23 +208,26 @@ void ManualModel::update_section_node_on_toggle(const GUI::ModelIndex& index, bo static_cast(node)->set_open(open); } -TriState ManualModel::data_matches(const GUI::ModelIndex& index, const GUI::Variant& term) const +GUI::Model::MatchResult ManualModel::data_matches(const GUI::ModelIndex& index, const GUI::Variant& term) const { auto name = page_name(index); if (!name.has_value()) - return TriState::False; + return { TriState::False }; auto match_result = fuzzy_match(term.as_string(), name.value()); if (match_result.score > 0) - return TriState::True; + return { TriState::True, match_result.score }; auto path = page_path(index); // NOTE: This is slightly inaccurate, as page_path can also fail due to OOM. We consider it acceptable to have a data mismatch in that case. if (!path.has_value()) - return TriState::False; + return { TriState::False }; auto view_result = page_view(path.release_value()); if (view_result.is_error() || view_result.value().is_empty()) - return TriState::False; + return { TriState::False }; - return view_result.value().contains(term.as_string(), CaseSensitivity::CaseInsensitive) ? TriState::True : TriState::False; + if (view_result.value().contains(term.as_string(), CaseSensitivity::CaseInsensitive)) + return { TriState::True, 0 }; + + return { TriState::False }; } diff --git a/Userland/Applications/Help/ManualModel.h b/Userland/Applications/Help/ManualModel.h index 41415d9605..526e8623ce 100644 --- a/Userland/Applications/Help/ManualModel.h +++ b/Userland/Applications/Help/ManualModel.h @@ -32,7 +32,7 @@ public: virtual int row_count(const GUI::ModelIndex& = GUI::ModelIndex()) const override; virtual int column_count(const GUI::ModelIndex& = GUI::ModelIndex()) const override; virtual GUI::Variant data(const GUI::ModelIndex&, GUI::ModelRole) const override; - virtual TriState data_matches(const GUI::ModelIndex&, const GUI::Variant&) const override; + virtual GUI::Model::MatchResult data_matches(const GUI::ModelIndex&, const GUI::Variant&) const override; virtual GUI::ModelIndex parent_index(const GUI::ModelIndex&) const override; virtual GUI::ModelIndex index(int row, int column = 0, const GUI::ModelIndex& parent = GUI::ModelIndex()) const override; diff --git a/Userland/Demos/ModelGallery/BasicModel.cpp b/Userland/Demos/ModelGallery/BasicModel.cpp index 464fb8fd5c..fe70c7133f 100644 --- a/Userland/Demos/ModelGallery/BasicModel.cpp +++ b/Userland/Demos/ModelGallery/BasicModel.cpp @@ -16,15 +16,19 @@ GUI::Variant BasicModel::data(GUI::ModelIndex const& index, GUI::ModelRole role) return m_items.at(index.row()); } -TriState BasicModel::data_matches(GUI::ModelIndex const& index, GUI::Variant const& data) const +GUI::Model::MatchResult BasicModel::data_matches(GUI::ModelIndex const& index, GUI::Variant const& data) const { if (!is_within_range(index)) - return TriState::False; + return { TriState::False }; if (!data.is_string()) - return TriState::False; + return { TriState::False }; auto& value = m_items.at(index.row()); - return value.contains(data.as_string()) ? TriState::True : TriState::False; + + if (value.contains(data.as_string())) + return { TriState::True }; + + return { TriState::False }; } void BasicModel::invalidate() diff --git a/Userland/Demos/ModelGallery/BasicModel.h b/Userland/Demos/ModelGallery/BasicModel.h index e682746b24..9e7fd9b581 100644 --- a/Userland/Demos/ModelGallery/BasicModel.h +++ b/Userland/Demos/ModelGallery/BasicModel.h @@ -23,7 +23,7 @@ public: virtual String column_name(int) const override { return "Item"_short_string; } virtual GUI::Variant data(GUI::ModelIndex const&, GUI::ModelRole = GUI::ModelRole::Display) const override; - virtual TriState data_matches(GUI::ModelIndex const&, GUI::Variant const&) const override; + virtual GUI::Model::MatchResult data_matches(GUI::ModelIndex const&, GUI::Variant const&) const override; virtual void invalidate() override; virtual GUI::ModelIndex index(int row, int column = 0, GUI::ModelIndex const& parent = GUI::ModelIndex()) const override; diff --git a/Userland/Libraries/LibGUI/CommandPalette.cpp b/Userland/Libraries/LibGUI/CommandPalette.cpp index cf08935fcf..2159835161 100644 --- a/Userland/Libraries/LibGUI/CommandPalette.cpp +++ b/Userland/Libraries/LibGUI/CommandPalette.cpp @@ -135,16 +135,17 @@ public: VERIFY_NOT_REACHED(); } - virtual TriState data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const override + virtual GUI::Model::MatchResult data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const override { auto needle = term.as_string(); if (needle.is_empty()) - return TriState::True; + return { TriState::True }; auto haystack = DeprecatedString::formatted("{} {}", menu_name(index), action_text(index)); - if (fuzzy_match(needle, haystack).score > 0) - return TriState::True; - return TriState::False; + auto match_result = fuzzy_match(needle, haystack); + if (match_result.score > 0) + return { TriState::True, match_result.score }; + return { TriState::False }; } static DeprecatedString action_text(ModelIndex const& index) diff --git a/Userland/Libraries/LibGUI/FilteringProxyModel.cpp b/Userland/Libraries/LibGUI/FilteringProxyModel.cpp index ae9f0f9529..e8097c5a3c 100644 --- a/Userland/Libraries/LibGUI/FilteringProxyModel.cpp +++ b/Userland/Libraries/LibGUI/FilteringProxyModel.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include namespace GUI { @@ -30,7 +31,7 @@ int FilteringProxyModel::column_count(ModelIndex const& index) const if ((size_t)index.row() > m_matching_indices.size() || index.row() < 0) return 0; - return m_model->column_count(m_matching_indices[index.row()]); + return m_model->column_count(m_matching_indices[index.row()].index); } String FilteringProxyModel::column_name(int column) const @@ -46,7 +47,7 @@ Variant FilteringProxyModel::data(ModelIndex const& index, ModelRole role) const if ((size_t)index.row() > m_matching_indices.size() || index.row() < 0) return {}; - auto matching_index = m_matching_indices[index.row()]; + auto matching_index = m_matching_indices[index.row()].index; auto underlying_index = m_model->index(matching_index.row(), index.column(), matching_index.parent()); return underlying_index.data(role); } @@ -67,15 +68,18 @@ void FilteringProxyModel::filter() if (!index.is_valid()) continue; - auto filter_matches = m_model->data_matches(index, m_filter_term); - bool matches = filter_matches == TriState::True; - if (filter_matches == TriState::Unknown) { + auto match_result = m_model->data_matches(index, m_filter_term); + bool matches = match_result.matched == TriState::True; + auto score = match_result.score; + if (match_result.matched == TriState::Unknown) { auto data = index.data(); - if (data.is_string() && data.as_string().contains(m_filter_term)) + if (data.is_string() && data.as_string().contains(m_filter_term)) { matches = true; + score = 0; + } } if (matches) - m_matching_indices.append(index); + m_matching_indices.append({ index, score }); add_matching(index); } @@ -83,6 +87,9 @@ void FilteringProxyModel::filter() ModelIndex parent_index; add_matching(parent_index); + if (has_flag(m_filtering_options, FilteringOptions::SortByScore)) + // Use a stable sort, so that indices with equal scores don't swap positions. + insertion_sort(m_matching_indices, [](auto const& a, auto const& b) { return b.score < a.score; }); } void FilteringProxyModel::set_filter_term(StringView term) @@ -100,7 +107,7 @@ ModelIndex FilteringProxyModel::map(ModelIndex const& index) const auto row = index.row(); if (m_matching_indices.size() > (size_t)row) - return m_matching_indices[row]; + return m_matching_indices[row].index; return {}; } diff --git a/Userland/Libraries/LibGUI/FilteringProxyModel.h b/Userland/Libraries/LibGUI/FilteringProxyModel.h index 7557cbe7b7..8c01b1aacd 100644 --- a/Userland/Libraries/LibGUI/FilteringProxyModel.h +++ b/Userland/Libraries/LibGUI/FilteringProxyModel.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include #include @@ -17,9 +18,14 @@ namespace GUI { class FilteringProxyModel final : public Model , public ModelClient { public: - static ErrorOr> create(NonnullRefPtr model) + enum class FilteringOptions { + None, + SortByScore = 1 << 1 + }; + + static ErrorOr> create(NonnullRefPtr model, FilteringOptions filtering_options = FilteringOptions::None) { - return adopt_nonnull_ref_or_enomem(new (nothrow) FilteringProxyModel(move(model))); + return adopt_nonnull_ref_or_enomem(new (nothrow) FilteringProxyModel(move(model), filtering_options)); } virtual ~FilteringProxyModel() override @@ -44,9 +50,15 @@ protected: virtual void model_did_update([[maybe_unused]] unsigned flags) override { invalidate(); } private: + struct ModelIndexWithScore { + ModelIndex index; + int score { 0 }; + }; + void filter(); - explicit FilteringProxyModel(NonnullRefPtr model) + explicit FilteringProxyModel(NonnullRefPtr model, FilteringOptions filtering_options = FilteringOptions::None) : m_model(move(model)) + , m_filtering_options(filtering_options) { m_model->register_client(*this); } @@ -54,9 +66,12 @@ private: NonnullRefPtr m_model; // Maps row to actual model index. - Vector m_matching_indices; + Vector m_matching_indices; DeprecatedString m_filter_term; + FilteringOptions m_filtering_options; }; +AK_ENUM_BITWISE_OPERATORS(FilteringProxyModel::FilteringOptions); + } diff --git a/Userland/Libraries/LibGUI/ItemListModel.h b/Userland/Libraries/LibGUI/ItemListModel.h index 049a7a36a9..db6b24c968 100644 --- a/Userland/Libraries/LibGUI/ItemListModel.h +++ b/Userland/Libraries/LibGUI/ItemListModel.h @@ -92,11 +92,11 @@ public: return {}; } - virtual TriState data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const override + virtual GUI::Model::MatchResult data_matches(GUI::ModelIndex const& index, GUI::Variant const& term) const override { if (index.data().as_string().contains(term.as_string(), CaseSensitivity::CaseInsensitive)) - return TriState::True; - return TriState::False; + return { TriState::True }; + return { TriState::False }; } virtual bool is_searchable() const override { return true; } diff --git a/Userland/Libraries/LibGUI/Model.h b/Userland/Libraries/LibGUI/Model.h index 51d3acaba1..fcf86035bc 100644 --- a/Userland/Libraries/LibGUI/Model.h +++ b/Userland/Libraries/LibGUI/Model.h @@ -62,13 +62,18 @@ public: MatchFull = 1 << 3, }; + struct MatchResult { + TriState matched { TriState::Unknown }; + int score { 0 }; + }; + virtual ~Model(); virtual int row_count(ModelIndex const& = ModelIndex()) const = 0; virtual int column_count(ModelIndex const& = ModelIndex()) const = 0; virtual String column_name(int) const { return {}; } virtual Variant data(ModelIndex const&, ModelRole = ModelRole::Display) const = 0; - virtual TriState data_matches(ModelIndex const&, Variant const&) const { return TriState::Unknown; } + virtual MatchResult data_matches(ModelIndex const&, Variant const&) const { return {}; } virtual void invalidate(); virtual ModelIndex parent_index(ModelIndex const&) const { return {}; } virtual ModelIndex index(int row, int column = 0, ModelIndex const& parent = ModelIndex()) const;