From ee7339b64bec9ced661cce63106665657090d0a1 Mon Sep 17 00:00:00 2001 From: Andrew <159703254+andrewsoonqn@users.noreply.github.com> Date: Mon, 6 Jul 2026 16:24:44 +0800 Subject: [PATCH] Refine plugin sorting to use name-first base order Ensure plugins are sorted alphabetically by name when no other column is sorted, and resolve ties by source, status, type, and plugin_key. --- src/slic3r/GUI/PluginSort.hpp | 16 +++++++------- tests/slic3rutils/test_plugin_sort.cpp | 30 ++++++++++++++------------ 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/slic3r/GUI/PluginSort.hpp b/src/slic3r/GUI/PluginSort.hpp index c75d0fdf53..d0a45cbd05 100644 --- a/src/slic3r/GUI/PluginSort.hpp +++ b/src/slic3r/GUI/PluginSort.hpp @@ -129,20 +129,20 @@ namespace Slic3r::GUI return li == lhs.size() ? -1 : 1; } - // Neutral baseline order used as the tie-breaker under every primary sort key. Always - // ascending: source priority, then type_key, then display_name, then plugin_key. - // e.g. two items with equal Status sort by source priority (Mine, then Subscribed, - // then Local), then by name. + // Neutral baseline order: the whole order when no column is sorted, and the tie-breaker under + // every primary sort key. Name-first so the default view is intuitively alphabetical: + // name, then source, then status, then type, with plugin_key as the final deterministic tie. + // e.g. with no column sorted the list reads A..Z by name. template int compare_plugin_base_order(const PluginItem& lhs, const PluginItem& rhs) { - // why: source ties use the declared PluginSource ordinal priority - the same order the - // Source sort key uses - so the neutral baseline never contradicts it. + if (const int cmp = compare_ascii_case_insensitive_natural(lhs.display_name, rhs.display_name); cmp != 0) + return cmp; if (const int cmp = static_cast(lhs.source) - static_cast(rhs.source); cmp != 0) return cmp; - if (const int cmp = lhs.type_key.compare(rhs.type_key); cmp != 0) + if (const int cmp = static_cast(lhs.status) - static_cast(rhs.status); cmp != 0) return cmp; - if (const int cmp = lhs.display_name.compare(rhs.display_name); cmp != 0) + if (const int cmp = lhs.type_key.compare(rhs.type_key); cmp != 0) return cmp; return lhs.plugin_key.compare(rhs.plugin_key); } diff --git a/tests/slic3rutils/test_plugin_sort.cpp b/tests/slic3rutils/test_plugin_sort.cpp index 2c1b3b7a2c..58db799dd8 100644 --- a/tests/slic3rutils/test_plugin_sort.cpp +++ b/tests/slic3rutils/test_plugin_sort.cpp @@ -49,11 +49,11 @@ TEST_CASE("plugin dialog status sort uses requested priority and base-order ties sort_plugin_items_for_dialog(items, PluginSortKey::Status, PluginSortOrder::Asc); - // why: local_activated and mine_activated tie on Status, so base order breaks the tie by - // declared source priority - Mine (ordinal 0) before Local (ordinal 2). + // why: local_activated and mine_activated tie on Status, so base order breaks the tie by name + // (case-insensitive) - "Local Activated" before "Mine Activated". const std::vector expected = { - "mine_activated", "local_activated", + "mine_activated", "mine_error", "local_inactive", "subscribed_loading", @@ -63,13 +63,13 @@ TEST_CASE("plugin dialog status sort uses requested priority and base-order ties sort_plugin_items_for_dialog(items, PluginSortKey::Status, PluginSortOrder::Desc); // why: Desc reverses the status ordinal, but the Activated tie still resolves by ascending - // base order (Mine before Local) - the direction only flips the primary key. + // base order (name: "Local Activated" before "Mine Activated") - direction only flips the key. const std::vector desc_expected = { "subscribed_loading", "local_inactive", "mine_error", - "mine_activated", "local_activated", + "mine_activated", }; CHECK(keys(items) == desc_expected); } @@ -121,14 +121,16 @@ TEST_CASE("plugin dialog name sort is case-insensitive and numeric-aware", "[plu sort_plugin_items_for_dialog(items, PluginSortKey::Name, PluginSortOrder::Asc); - const std::vector expected = {"ada_upper", "ada_lower", "rig2", "rig10"}; + // why: "Ada"/"ada" tie on the case-insensitive name (primary AND base name level), so the tie + // falls through source/status/type to plugin_key: "ada_lower" before "ada_upper". + const std::vector expected = {"ada_lower", "ada_upper", "rig2", "rig10"}; CHECK(keys(items) == expected); sort_plugin_items_for_dialog(items, PluginSortKey::Name, PluginSortOrder::Desc); // why: names reverse ("Rig 10" before "Rig 2"), but "Ada"/"ada" tie on the case-insensitive - // key and keep ascending base order (case-sensitive "Ada" < "ada"). - const std::vector desc_expected = {"rig10", "rig2", "ada_upper", "ada_lower"}; + // key and keep ascending base order, which resolves by plugin_key ("ada_lower" < "ada_upper"). + const std::vector desc_expected = {"rig10", "rig2", "ada_lower", "ada_upper"}; CHECK(keys(items) == desc_expected); } @@ -159,14 +161,14 @@ TEST_CASE("natural compare handles digits, case, prefixes and leading zeros", "[ TEST_CASE("plugin dialog None sort key falls to ascending base order in both directions", "[plugin][sort]") { std::vector items = { - {"local_z", PluginSource::Local, PluginStatus::Activated, "script", "Zeta"}, - {"mine_a", PluginSource::Mine, PluginStatus::Inactive, "script", "Alpha"}, - {"sub_m", PluginSource::Subscribed, PluginStatus::Error, "script", "Mu"}, + {"z_mine", PluginSource::Mine, PluginStatus::Activated, "script", "Zebra"}, + {"a_local", PluginSource::Local, PluginStatus::Activated, "script", "Apple"}, + {"m_sub", PluginSource::Subscribed, PluginStatus::Error, "script", "Mango"}, }; - // why: base order is source priority (Mine, Subscribed, Local) then name - and it ignores the - // requested status/order entirely, so the neutral baseline is deterministic. - const std::vector base_expected = {"mine_a", "sub_m", "local_z"}; + // why: no primary key -> pure name-first base order (Apple < Mango < Zebra). A source-first + // baseline would instead give {z_mine, m_sub, a_local}, so this pins the name-first order. + const std::vector base_expected = {"a_local", "m_sub", "z_mine"}; sort_plugin_items_for_dialog(items, PluginSortKey::None, PluginSortOrder::Asc); CHECK(keys(items) == base_expected);