From 7c87534a9f9826b4c8e0bc08676385fe5190be64 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Fri, 24 Apr 2026 11:55:54 -0700 Subject: [PATCH 1/7] [OutputSort, Settings] Add sort field and direction types with settings infrastructure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Introduce configurable output sorting for `winget list` as part of issue the sorting feature request. This is part 1 of a 3-PR stack that lays the type system and settings foundation without changing any runtime behavior. ## Changes - **SortField and SortDirection enums**: Add `SortField` (Relevance, Name, Id, Version, Source, Available) and `SortDirection` (Ascending, Descending) to the settings type system. Relevance is internal-only and not exposed as a valid settings value. - **Settings entries**: Register `OutputSortOrder` and `OutputSortDirection` with typed validation that maps JSON string values to enums using case-insensitive comparison. Default sort order is `[Name]` ascending. Invalid field names reject the entire setting and fall back to default. - **JSON schema**: Extend `settings.schema.0.2.json` with an `output` section containing `sortOrder` (string array) and `sortDirection` (string enum with ascending default). - **Unit tests**: Add 16 test sections covering default values, single and multi-field configurations, case insensitivity, empty array (disables sorting), invalid fields, mixed valid/invalid, and wrong JSON types. ## Impact - No behavioral change ╬ô├ç├╢ settings are defined but not yet consumed by any command or workflow. Existing `winget list` output remains unchanged. - Users who edit `settings.json` to add `output.sortOrder` or `output.sortDirection` will have their values validated and stored, but the values will not take effect until PR3 wires the sort logic. ## How Validated - Unit tests added (16 test sections across SettingOutputSortOrder and SettingOutputSortDirection test cases) - Compilation verified via msbuild for AppInstallerCommonCore and AppInstallerCLITests (compile-only) --- .../JSON/settings/settings.schema.0.2.json | 35 ++++ src/AppInstallerCLITests/UserSettings.cpp | 178 ++++++++++++++++++ .../Public/winget/UserSettings.h | 24 +++ src/AppInstallerCommonCore/UserSettings.cpp | 47 +++++ 4 files changed, 284 insertions(+) diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index 25b04e3d1f..afbf8adbdf 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -340,6 +340,35 @@ "default": false } } + }, + "Output": { + "description": "Output display settings", + "type": "object", + "properties": { + "sortOrder": { + "description": "Fields to sort output by, in priority order. Use an empty array to disable sorting.", + "type": "array", + "items": { + "type": "string", + "enum": [ + "name", + "id", + "version", + "source", + "available" + ] + } + }, + "sortDirection": { + "description": "Sort direction for output ordering", + "type": "string", + "enum": [ + "ascending", + "descending" + ], + "default": "ascending" + } + } } }, "allOf": [ @@ -409,6 +438,12 @@ }, "additionalItems": true }, + { + "properties": { + "output": { "$ref": "#/definitions/Output" } + }, + "additionalItems": true + }, { "properties": { "experimentalFeatures": { "$ref": "#/definitions/Experimental" } diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp index 3d0820f8d5..a472287172 100644 --- a/src/AppInstallerCLITests/UserSettings.cpp +++ b/src/AppInstallerCLITests/UserSettings.cpp @@ -660,3 +660,181 @@ TEST_CASE("LoggingChannels", "[settings]") REQUIRE(userSettingTest.Get() == (Channel::CLI | Channel::SQL)); } } + +TEST_CASE("SettingOutputSortOrder", "[settings]") +{ + auto again = DeleteUserSettingsFiles(); + + SECTION("Default value") + { + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 1); + REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Single field") + { + std::string_view json = R"({ "output": { "sortOrder": ["id"] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 1); + REQUIRE(sortOrder[0] == SortField::Id); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Multiple fields") + { + std::string_view json = R"({ "output": { "sortOrder": ["available", "name"] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 2); + REQUIRE(sortOrder[0] == SortField::Available); + REQUIRE(sortOrder[1] == SortField::Name); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("All valid fields") + { + std::string_view json = R"({ "output": { "sortOrder": ["name", "id", "version", "source", "available"] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 5); + REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(sortOrder[1] == SortField::Id); + REQUIRE(sortOrder[2] == SortField::Version); + REQUIRE(sortOrder[3] == SortField::Source); + REQUIRE(sortOrder[4] == SortField::Available); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Case insensitive") + { + std::string_view json = R"({ "output": { "sortOrder": ["Name", "ID", "VERSION"] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 3); + REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(sortOrder[1] == SortField::Id); + REQUIRE(sortOrder[2] == SortField::Version); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Empty array disables sorting") + { + std::string_view json = R"({ "output": { "sortOrder": [] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 0); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Invalid field name") + { + std::string_view json = R"({ "output": { "sortOrder": ["invalid"] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 1); + REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(userSettingTest.GetWarnings().size() == 1); + } + SECTION("Mixed valid and invalid field") + { + std::string_view json = R"({ "output": { "sortOrder": ["name", "bogus"] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 1); + REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(userSettingTest.GetWarnings().size() == 1); + } + SECTION("Wrong type - string instead of array") + { + std::string_view json = R"({ "output": { "sortOrder": "name" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 1); + REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(userSettingTest.GetWarnings().size() == 1); + } + SECTION("Wrong type - number in array") + { + std::string_view json = R"({ "output": { "sortOrder": [1] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 1); + REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(userSettingTest.GetWarnings().size() == 1); + } +} + +TEST_CASE("SettingOutputSortDirection", "[settings]") +{ + auto again = DeleteUserSettingsFiles(); + + SECTION("Default value") + { + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == SortDirection::Ascending); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Ascending") + { + std::string_view json = R"({ "output": { "sortDirection": "ascending" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == SortDirection::Ascending); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Descending") + { + std::string_view json = R"({ "output": { "sortDirection": "descending" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == SortDirection::Descending); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Case insensitive") + { + std::string_view json = R"({ "output": { "sortDirection": "DESCENDING" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == SortDirection::Descending); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } + SECTION("Invalid value") + { + std::string_view json = R"({ "output": { "sortDirection": "sideways" } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == SortDirection::Ascending); + REQUIRE(userSettingTest.GetWarnings().size() == 1); + } + SECTION("Wrong type - array instead of string") + { + std::string_view json = R"({ "output": { "sortDirection": ["ascending"] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + REQUIRE(userSettingTest.Get() == SortDirection::Ascending); + REQUIRE(userSettingTest.GetWarnings().size() == 1); + } +} diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index 95fbe73549..ae171bf7f1 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -47,6 +47,24 @@ namespace AppInstaller::Settings Disabled, }; + // Sort field for output ordering. + enum class SortField + { + Relevance, // Internal: preserves current natural order (not valid in settings) + Name, + Id, + Version, + Source, + Available, + }; + + // Sort direction for output ordering. + enum class SortDirection + { + Ascending, + Descending, + }; + // The download code to use for *installers*. enum class InstallerDownloader { @@ -114,6 +132,9 @@ namespace AppInstaller::Settings ConfigureDefaultModuleRoot, // Interactivity InteractivityDisable, + // Output behavior + OutputSortOrder, + OutputSortDirection, #ifndef AICLI_DISABLE_TEST_HOOKS // Debug EnableSelfInitiatedMinidump, @@ -208,6 +229,9 @@ namespace AppInstaller::Settings SETTINGMAPPING_SPECIALIZATION(Setting::LoggingFileCountLimit, uint32_t, uint32_t, 0, ".logging.file.countLimit"sv); // Interactivity SETTINGMAPPING_SPECIALIZATION(Setting::InteractivityDisable, bool, bool, false, ".interactivity.disable"sv); + // Output behavior + SETTINGMAPPING_SPECIALIZATION(Setting::OutputSortOrder, std::vector, std::vector, std::vector{ SortField::Name }, ".output.sortOrder"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::OutputSortDirection, std::string, SortDirection, SortDirection::Ascending, ".output.sortDirection"sv); // Used to deduce the SettingVariant type; making a variant that includes std::monostate and all SettingMapping types. template diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index db276351ff..d8351847f0 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -482,6 +482,53 @@ namespace AppInstaller::Settings { return value * 24h; } + + WINGET_VALIDATE_SIGNATURE(OutputSortOrder) + { + std::vector fields; + for (auto const& entry : value) + { + if (Utility::CaseInsensitiveEquals(entry, "name")) + { + fields.emplace_back(SortField::Name); + } + else if (Utility::CaseInsensitiveEquals(entry, "id")) + { + fields.emplace_back(SortField::Id); + } + else if (Utility::CaseInsensitiveEquals(entry, "version")) + { + fields.emplace_back(SortField::Version); + } + else if (Utility::CaseInsensitiveEquals(entry, "source")) + { + fields.emplace_back(SortField::Source); + } + else if (Utility::CaseInsensitiveEquals(entry, "available")) + { + fields.emplace_back(SortField::Available); + } + else + { + return {}; + } + } + return fields; + } + + WINGET_VALIDATE_SIGNATURE(OutputSortDirection) + { + if (Utility::CaseInsensitiveEquals(value, "ascending")) + { + return SortDirection::Ascending; + } + else if (Utility::CaseInsensitiveEquals(value, "descending")) + { + return SortDirection::Descending; + } + + return {}; + } } #ifndef AICLI_DISABLE_TEST_HOOKS From f30111529cb5963509549416caa48345b08eca04 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Fri, 24 Apr 2026 17:15:48 -0700 Subject: [PATCH 2/7] [OutputSort, Settings] Use static constexpr string views in sort validators Follow established codebase pattern (NetworkDownloader, LoggingLevel) of declaring static constexpr std::string_view variables for string comparisons instead of inline string literals. --- src/AppInstallerCommonCore/UserSettings.cpp | 23 ++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index d8351847f0..9cdad999e5 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -485,26 +485,32 @@ namespace AppInstaller::Settings WINGET_VALIDATE_SIGNATURE(OutputSortOrder) { + static constexpr std::string_view s_sortField_name = "name"; + static constexpr std::string_view s_sortField_id = "id"; + static constexpr std::string_view s_sortField_version = "version"; + static constexpr std::string_view s_sortField_source = "source"; + static constexpr std::string_view s_sortField_available = "available"; + std::vector fields; for (auto const& entry : value) { - if (Utility::CaseInsensitiveEquals(entry, "name")) + if (Utility::CaseInsensitiveEquals(entry, s_sortField_name)) { fields.emplace_back(SortField::Name); } - else if (Utility::CaseInsensitiveEquals(entry, "id")) + else if (Utility::CaseInsensitiveEquals(entry, s_sortField_id)) { fields.emplace_back(SortField::Id); } - else if (Utility::CaseInsensitiveEquals(entry, "version")) + else if (Utility::CaseInsensitiveEquals(entry, s_sortField_version)) { fields.emplace_back(SortField::Version); } - else if (Utility::CaseInsensitiveEquals(entry, "source")) + else if (Utility::CaseInsensitiveEquals(entry, s_sortField_source)) { fields.emplace_back(SortField::Source); } - else if (Utility::CaseInsensitiveEquals(entry, "available")) + else if (Utility::CaseInsensitiveEquals(entry, s_sortField_available)) { fields.emplace_back(SortField::Available); } @@ -518,11 +524,14 @@ namespace AppInstaller::Settings WINGET_VALIDATE_SIGNATURE(OutputSortDirection) { - if (Utility::CaseInsensitiveEquals(value, "ascending")) + static constexpr std::string_view s_sortDirection_ascending = "ascending"; + static constexpr std::string_view s_sortDirection_descending = "descending"; + + if (Utility::CaseInsensitiveEquals(value, s_sortDirection_ascending)) { return SortDirection::Ascending; } - else if (Utility::CaseInsensitiveEquals(value, "descending")) + else if (Utility::CaseInsensitiveEquals(value, s_sortDirection_descending)) { return SortDirection::Descending; } From 184cacd7151c3b416e7a0abb5dedbb65143f26ce Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Mon, 27 Apr 2026 10:00:26 -0700 Subject: [PATCH 3/7] [Tests, OutputSort] Add test for duplicate sort field values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add test case verifying behavior when duplicate field names appear in sortOrder settings (e.g., ["name", "id", "Name"]). Duplicates are accepted and preserved — case-insensitive matching maps both "name" and "Name" to SortField::Name. Addresses reviewer feedback from PR #6176. --- src/AppInstallerCLITests/UserSettings.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp index a472287172..a514399c78 100644 --- a/src/AppInstallerCLITests/UserSettings.cpp +++ b/src/AppInstallerCLITests/UserSettings.cpp @@ -757,6 +757,19 @@ TEST_CASE("SettingOutputSortOrder", "[settings]") REQUIRE(sortOrder[0] == SortField::Name); REQUIRE(userSettingTest.GetWarnings().size() == 1); } + SECTION("Duplicate values - case-insensitive") + { + std::string_view json = R"({ "output": { "sortOrder": ["name", "id", "Name"] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 3); + REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(sortOrder[1] == SortField::Id); + REQUIRE(sortOrder[2] == SortField::Name); + REQUIRE(userSettingTest.GetWarnings().empty()); + } SECTION("Wrong type - string instead of array") { std::string_view json = R"({ "output": { "sortOrder": "name" } })"; From c7639c18026617cadff4496fa6e390cb2696448e Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Mon, 27 Apr 2026 11:28:59 -0700 Subject: [PATCH 4/7] [OutputSort, Settings] Address review: empty default sentinel, public relevance, lowercase-once - Change OutputSortOrder default from [Name] to empty vector (sentinel for context-aware defaults: relevance when query, Name when no query) - Make Relevance a publicly visible sort field in settings and schema - Refactor OutputSortOrder validator to ToLower once then compare with == - Update tests for new default, add relevance field test section --- .../JSON/settings/settings.schema.0.2.json | 1 + src/AppInstallerCLITests/UserSettings.cpp | 43 +++++++++++-------- .../Public/winget/UserSettings.h | 4 +- src/AppInstallerCommonCore/UserSettings.cpp | 17 +++++--- 4 files changed, 40 insertions(+), 25 deletions(-) diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index afbf8adbdf..529ae05828 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -351,6 +351,7 @@ "items": { "type": "string", "enum": [ + "relevance", "name", "id", "version", diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp index a514399c78..c84a1891d3 100644 --- a/src/AppInstallerCLITests/UserSettings.cpp +++ b/src/AppInstallerCLITests/UserSettings.cpp @@ -665,13 +665,12 @@ TEST_CASE("SettingOutputSortOrder", "[settings]") { auto again = DeleteUserSettingsFiles(); - SECTION("Default value") + SECTION("Default value - empty vector sentinel for context-aware defaults") { UserSettingsTest userSettingTest; auto sortOrder = userSettingTest.Get(); - REQUIRE(sortOrder.size() == 1); - REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(sortOrder.empty()); REQUIRE(userSettingTest.GetWarnings().size() == 0); } SECTION("Single field") @@ -685,6 +684,17 @@ TEST_CASE("SettingOutputSortOrder", "[settings]") REQUIRE(sortOrder[0] == SortField::Id); REQUIRE(userSettingTest.GetWarnings().size() == 0); } + SECTION("Relevance field") + { + std::string_view json = R"({ "output": { "sortOrder": ["relevance"] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto sortOrder = userSettingTest.Get(); + REQUIRE(sortOrder.size() == 1); + REQUIRE(sortOrder[0] == SortField::Relevance); + REQUIRE(userSettingTest.GetWarnings().size() == 0); + } SECTION("Multiple fields") { std::string_view json = R"({ "output": { "sortOrder": ["available", "name"] } })"; @@ -699,17 +709,18 @@ TEST_CASE("SettingOutputSortOrder", "[settings]") } SECTION("All valid fields") { - std::string_view json = R"({ "output": { "sortOrder": ["name", "id", "version", "source", "available"] } })"; + std::string_view json = R"({ "output": { "sortOrder": ["relevance", "name", "id", "version", "source", "available"] } })"; SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; auto sortOrder = userSettingTest.Get(); - REQUIRE(sortOrder.size() == 5); - REQUIRE(sortOrder[0] == SortField::Name); - REQUIRE(sortOrder[1] == SortField::Id); - REQUIRE(sortOrder[2] == SortField::Version); - REQUIRE(sortOrder[3] == SortField::Source); - REQUIRE(sortOrder[4] == SortField::Available); + REQUIRE(sortOrder.size() == 6); + REQUIRE(sortOrder[0] == SortField::Relevance); + REQUIRE(sortOrder[1] == SortField::Name); + REQUIRE(sortOrder[2] == SortField::Id); + REQUIRE(sortOrder[3] == SortField::Version); + REQUIRE(sortOrder[4] == SortField::Source); + REQUIRE(sortOrder[5] == SortField::Available); REQUIRE(userSettingTest.GetWarnings().size() == 0); } SECTION("Case insensitive") @@ -742,8 +753,7 @@ TEST_CASE("SettingOutputSortOrder", "[settings]") UserSettingsTest userSettingTest; auto sortOrder = userSettingTest.Get(); - REQUIRE(sortOrder.size() == 1); - REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(sortOrder.empty()); REQUIRE(userSettingTest.GetWarnings().size() == 1); } SECTION("Mixed valid and invalid field") @@ -753,8 +763,7 @@ TEST_CASE("SettingOutputSortOrder", "[settings]") UserSettingsTest userSettingTest; auto sortOrder = userSettingTest.Get(); - REQUIRE(sortOrder.size() == 1); - REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(sortOrder.empty()); REQUIRE(userSettingTest.GetWarnings().size() == 1); } SECTION("Duplicate values - case-insensitive") @@ -777,8 +786,7 @@ TEST_CASE("SettingOutputSortOrder", "[settings]") UserSettingsTest userSettingTest; auto sortOrder = userSettingTest.Get(); - REQUIRE(sortOrder.size() == 1); - REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(sortOrder.empty()); REQUIRE(userSettingTest.GetWarnings().size() == 1); } SECTION("Wrong type - number in array") @@ -788,8 +796,7 @@ TEST_CASE("SettingOutputSortOrder", "[settings]") UserSettingsTest userSettingTest; auto sortOrder = userSettingTest.Get(); - REQUIRE(sortOrder.size() == 1); - REQUIRE(sortOrder[0] == SortField::Name); + REQUIRE(sortOrder.empty()); REQUIRE(userSettingTest.GetWarnings().size() == 1); } } diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index ae171bf7f1..94db508211 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -50,7 +50,7 @@ namespace AppInstaller::Settings // Sort field for output ordering. enum class SortField { - Relevance, // Internal: preserves current natural order (not valid in settings) + Relevance, // Preserves current natural order (source-defined relevance ranking) Name, Id, Version, @@ -230,7 +230,7 @@ namespace AppInstaller::Settings // Interactivity SETTINGMAPPING_SPECIALIZATION(Setting::InteractivityDisable, bool, bool, false, ".interactivity.disable"sv); // Output behavior - SETTINGMAPPING_SPECIALIZATION(Setting::OutputSortOrder, std::vector, std::vector, std::vector{ SortField::Name }, ".output.sortOrder"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::OutputSortOrder, std::vector, std::vector, std::vector{}, ".output.sortOrder"sv); SETTINGMAPPING_SPECIALIZATION(Setting::OutputSortDirection, std::string, SortDirection, SortDirection::Ascending, ".output.sortDirection"sv); // Used to deduce the SettingVariant type; making a variant that includes std::monostate and all SettingMapping types. diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index 9cdad999e5..bfa7d29567 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -485,6 +485,7 @@ namespace AppInstaller::Settings WINGET_VALIDATE_SIGNATURE(OutputSortOrder) { + static constexpr std::string_view s_sortField_relevance = "relevance"; static constexpr std::string_view s_sortField_name = "name"; static constexpr std::string_view s_sortField_id = "id"; static constexpr std::string_view s_sortField_version = "version"; @@ -494,23 +495,29 @@ namespace AppInstaller::Settings std::vector fields; for (auto const& entry : value) { - if (Utility::CaseInsensitiveEquals(entry, s_sortField_name)) + std::string lowered = Utility::ToLower(entry); + + if (lowered == s_sortField_relevance) + { + fields.emplace_back(SortField::Relevance); + } + else if (lowered == s_sortField_name) { fields.emplace_back(SortField::Name); } - else if (Utility::CaseInsensitiveEquals(entry, s_sortField_id)) + else if (lowered == s_sortField_id) { fields.emplace_back(SortField::Id); } - else if (Utility::CaseInsensitiveEquals(entry, s_sortField_version)) + else if (lowered == s_sortField_version) { fields.emplace_back(SortField::Version); } - else if (Utility::CaseInsensitiveEquals(entry, s_sortField_source)) + else if (lowered == s_sortField_source) { fields.emplace_back(SortField::Source); } - else if (Utility::CaseInsensitiveEquals(entry, s_sortField_available)) + else if (lowered == s_sortField_available) { fields.emplace_back(SortField::Available); } From f15a3e1c855a29bfc8b72f0ba0e6993c6c2e7d32 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Fri, 24 Apr 2026 13:22:40 -0700 Subject: [PATCH 5/7] [ListCommand, Args] Register --sort, --ascending, and --descending arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Register CLI arguments for output sorting on the list command as part of the sorting feature request. This is part 2 of a 3-PR stack that adds the argument definitions without wiring them to any sort logic. ## Changes - **Argument types**: Add Sort, SortAscending, and SortDescending to the Args::Type enum under the List Command section. - **Argument registration**: Map Sort to `--sort` (Standard arg), SortAscending to `--ascending`/`--asc` (Flag), and SortDescending to `--descending`/`--desc` (Flag) in ArgumentCommon::ForType. Ascending and descending share a SortDirection exclusive set to enforce mutual exclusivity. - **ListCommand integration**: Add all three arguments to ListCommand::GetArguments() with descriptive resource strings. - **Resource strings**: Add SortArgumentDescription, SortAscendingArgumentDescription, and SortDescendingArgumentDescription with localization comments to winget.resw and corresponding IDs to Resources.h. ## Impact - No behavioral change ╬ô├ç├╢ arguments are accepted and parsed but not consumed by any workflow. Existing `winget list` output is unchanged. - The existing EnsureAllArgumentsDefined test will automatically verify all three new arg types have valid ForType mappings. - `--ascending` and `--descending` cannot be used together due to the SortDirection exclusive set constraint. ## How Validated - Compilation verified via msbuild for AppInstallerCLICore (full build) and AppInstallerCLITests (compile-only) --- src/AppInstallerCLICore/Argument.cpp | 6 ++++++ src/AppInstallerCLICore/Argument.h | 1 + src/AppInstallerCLICore/Commands/ListCommand.cpp | 3 +++ src/AppInstallerCLICore/ExecutionArgs.h | 3 +++ src/AppInstallerCLICore/Resources.h | 3 +++ .../Shared/Strings/en-us/winget.resw | 12 ++++++++++++ 6 files changed, 28 insertions(+) diff --git a/src/AppInstallerCLICore/Argument.cpp b/src/AppInstallerCLICore/Argument.cpp index 32bc8651c7..99699bdb0a 100644 --- a/src/AppInstallerCLICore/Argument.cpp +++ b/src/AppInstallerCLICore/Argument.cpp @@ -189,6 +189,12 @@ namespace AppInstaller::CLI return { type, "upgrade-available"_liv}; case Execution::Args::Type::ListDetails: return { type, "details"_liv }; + case Execution::Args::Type::Sort: + return { type, "sort"_liv }; + case Execution::Args::Type::SortAscending: + return { type, "ascending"_liv, "asc"_liv, ArgTypeCategory::None, ArgTypeExclusiveSet::SortDirection }; + case Execution::Args::Type::SortDescending: + return { type, "descending"_liv, "desc"_liv, ArgTypeCategory::None, ArgTypeExclusiveSet::SortDirection }; // Pin command case Execution::Args::Type::GatedVersion: diff --git a/src/AppInstallerCLICore/Argument.h b/src/AppInstallerCLICore/Argument.h index e7b438f1a7..c1a573db6a 100644 --- a/src/AppInstallerCLICore/Argument.h +++ b/src/AppInstallerCLICore/Argument.h @@ -92,6 +92,7 @@ namespace AppInstaller::CLI ConfigurationSetChoice = 0x80, DscResourceFunction = 0x100, DependenciesConflict = 0x200, + SortDirection = 0x400, // This must always be at the end Max diff --git a/src/AppInstallerCLICore/Commands/ListCommand.cpp b/src/AppInstallerCLICore/Commands/ListCommand.cpp index 334ddb56c9..acb12bdd46 100644 --- a/src/AppInstallerCLICore/Commands/ListCommand.cpp +++ b/src/AppInstallerCLICore/Commands/ListCommand.cpp @@ -32,6 +32,9 @@ namespace AppInstaller::CLI Argument{ Execution::Args::Type::IncludeUnknown, Resource::String::IncludeUnknownInListArgumentDescription, ArgumentType::Flag }, Argument{ Execution::Args::Type::IncludePinned, Resource::String::IncludePinnedInListArgumentDescription, ArgumentType::Flag}, Argument::ForType(Execution::Args::Type::ListDetails), + Argument{ Execution::Args::Type::Sort, Resource::String::SortArgumentDescription, ArgumentType::Standard }, + Argument{ Execution::Args::Type::SortAscending, Resource::String::SortAscendingArgumentDescription, ArgumentType::Flag }, + Argument{ Execution::Args::Type::SortDescending, Resource::String::SortDescendingArgumentDescription, ArgumentType::Flag }, }; } diff --git a/src/AppInstallerCLICore/ExecutionArgs.h b/src/AppInstallerCLICore/ExecutionArgs.h index fa7e9c6358..675576a9d7 100644 --- a/src/AppInstallerCLICore/ExecutionArgs.h +++ b/src/AppInstallerCLICore/ExecutionArgs.h @@ -114,6 +114,9 @@ namespace AppInstaller::CLI::Execution // List Command Upgrade, // Used in List command to only show versions with upgrades ListDetails, + Sort, // Sort output by field (repeatable: --sort name --sort id) + SortAscending, // Sort output in ascending order + SortDescending, // Sort output in descending order // Pin command GatedVersion, // Differs from Version in that this supports wildcards diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 11b4b8d1a7..fa369b8ae5 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -685,6 +685,9 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(ShowVersion); WINGET_DEFINE_RESOURCE_STRINGID(SilentArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(SingleCharAfterDashError); + WINGET_DEFINE_RESOURCE_STRINGID(SortArgumentDescription); + WINGET_DEFINE_RESOURCE_STRINGID(SortAscendingArgumentDescription); + WINGET_DEFINE_RESOURCE_STRINGID(SortDescendingArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(SkipDependenciesArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(DependenciesOnlyArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(DependenciesOnlyMessage); diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 7293ac52a6..3e51c8589b 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -524,6 +524,18 @@ They can be configured through the settings file 'winget settings'. Only the single character alias can occur after a single -: '{0}' {Locked="{0}"} Error message displayed when the user provides more than a single character command line alias argument after an alias argument specifier '-'. {0} is a placeholder replaced by the user's argument input. + + Sort results by a specified field (can be repeated for multi-field sort) + Description for the --sort argument used to sort output by field name. Valid values are: name, id, version, source, available. + + + Sort results in ascending order + Description for the --ascending (--asc) flag that sets ascending sort direction for output. + + + Sort results in descending order + Description for the --descending (--desc) flag that sets descending sort direction for output. + A source with the given name already exists and refers to a different location: From 3d9c1e4f72e6c33aa656cbb2d4920fad121c4bdf Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Mon, 27 Apr 2026 17:14:47 -0700 Subject: [PATCH 6/7] Add ConvertToSortField public utility function Add a shared string-to-SortField converter so CLI argument parsing and workflow logic can resolve sort field names consistently. - Declare ConvertToSortField in UserSettings.h (public API) - Implement with case-insensitive matching via Utility::ToLower - Add Catch2 tests: lowercase, case-insensitive, invalid, round-trip Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/AppInstallerCLITests/UserSettings.cpp | 42 +++++++++++++++++++ .../Public/winget/UserSettings.h | 3 ++ src/AppInstallerCommonCore/UserSettings.cpp | 20 +++++++++ 3 files changed, 65 insertions(+) diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp index c84a1891d3..dfe2a4ba54 100644 --- a/src/AppInstallerCLITests/UserSettings.cpp +++ b/src/AppInstallerCLITests/UserSettings.cpp @@ -858,3 +858,45 @@ TEST_CASE("SettingOutputSortDirection", "[settings]") REQUIRE(userSettingTest.GetWarnings().size() == 1); } } + +TEST_CASE("ConvertToSortField", "[settings]") +{ + SECTION("Valid values - lowercase") + { + REQUIRE(ConvertToSortField("relevance") == SortField::Relevance); + REQUIRE(ConvertToSortField("name") == SortField::Name); + REQUIRE(ConvertToSortField("id") == SortField::Id); + REQUIRE(ConvertToSortField("version") == SortField::Version); + REQUIRE(ConvertToSortField("source") == SortField::Source); + REQUIRE(ConvertToSortField("available") == SortField::Available); + } + SECTION("Case-insensitive") + { + REQUIRE(ConvertToSortField("RELEVANCE") == SortField::Relevance); + REQUIRE(ConvertToSortField("NAME") == SortField::Name); + REQUIRE(ConvertToSortField("Id") == SortField::Id); + REQUIRE(ConvertToSortField("VERSION") == SortField::Version); + REQUIRE(ConvertToSortField("Source") == SortField::Source); + REQUIRE(ConvertToSortField("AVAILABLE") == SortField::Available); + } + SECTION("Invalid values return nullopt") + { + REQUIRE_FALSE(ConvertToSortField("").has_value()); + REQUIRE_FALSE(ConvertToSortField("foo").has_value()); + REQUIRE_FALSE(ConvertToSortField("names").has_value()); + REQUIRE_FALSE(ConvertToSortField("nam").has_value()); + } + SECTION("Settings round-trip with ConvertToSortField") + { + auto again = DeleteUserSettingsFiles(); + + std::string_view json = R"({ "output": { "sortOrder": ["name", "id"] } })"; + SetSetting(Stream::PrimaryUserSettings, json); + UserSettingsTest userSettingTest; + + auto fields = userSettingTest.Get(); + REQUIRE(fields.size() == 2); + REQUIRE(fields[0] == SortField::Name); + REQUIRE(fields[1] == SortField::Id); + } +} diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h index 94db508211..3fbf1f2952 100644 --- a/src/AppInstallerCommonCore/Public/winget/UserSettings.h +++ b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -65,6 +65,9 @@ namespace AppInstaller::Settings Descending, }; + // Converts a string to SortField. Returns std::nullopt for unrecognized values. + std::optional ConvertToSortField(std::string_view value); + // The download code to use for *installers*. enum class InstallerDownloader { diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index bfa7d29567..f15883be34 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -215,6 +215,26 @@ namespace AppInstaller::Settings } } + std::optional ConvertToSortField(std::string_view value) + { + static constexpr std::string_view s_sortField_relevance = "relevance"; + static constexpr std::string_view s_sortField_name = "name"; + static constexpr std::string_view s_sortField_id = "id"; + static constexpr std::string_view s_sortField_version = "version"; + static constexpr std::string_view s_sortField_source = "source"; + static constexpr std::string_view s_sortField_available = "available"; + + std::string lowered = Utility::ToLower(value); + + if (lowered == s_sortField_relevance) return SortField::Relevance; + if (lowered == s_sortField_name) return SortField::Name; + if (lowered == s_sortField_id) return SortField::Id; + if (lowered == s_sortField_version) return SortField::Version; + if (lowered == s_sortField_source) return SortField::Source; + if (lowered == s_sortField_available) return SortField::Available; + return std::nullopt; + } + namespace details { #define WINGET_VALIDATE_SIGNATURE(_setting_) \ From 19d65c8b0d8ea1e49c4b9700532fa871aa5107c8 Mon Sep 17 00:00:00 2001 From: Madhusudhan Gumbalapura Sudarshan Date: Tue, 28 Apr 2026 10:44:33 -0700 Subject: [PATCH 7/7] [UserSettings, Schema] Address PR review feedback for sort settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem - Schema description for `sortOrder` used "disable sorting" which didn't accurately describe the behavior of an empty array - The `--sort` argument description in winget.resw was unnecessarily verbose and included implementation details in the comment - The `OutputSortOrder` validator duplicated the same field-name-to-enum parsing logic that `ConvertToSortField` already provides ## Solution - Update schema description to "An empty array results in default sorting" - Shorten sort argument description to "Sort results by a property (can be repeated)" and simplify the comment - Refactor `OutputSortOrder` validator to call `ConvertToSortField()` instead of maintaining its own inline if/else chain with local constexpr strings ## Impact - Eliminates ~30 lines of duplicated string-to-enum parsing logic in the validator - Future sort field additions only need to update `ConvertToSortField` — the validator follows automatically Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../JSON/settings/settings.schema.0.2.json | 2 +- .../Shared/Strings/en-us/winget.resw | 4 +- src/AppInstallerCommonCore/UserSettings.cpp | 37 ++----------------- 3 files changed, 6 insertions(+), 37 deletions(-) diff --git a/schemas/JSON/settings/settings.schema.0.2.json b/schemas/JSON/settings/settings.schema.0.2.json index 529ae05828..d19a9fa25d 100644 --- a/schemas/JSON/settings/settings.schema.0.2.json +++ b/schemas/JSON/settings/settings.schema.0.2.json @@ -346,7 +346,7 @@ "type": "object", "properties": { "sortOrder": { - "description": "Fields to sort output by, in priority order. Use an empty array to disable sorting.", + "description": "Fields to sort output by, in priority order. An empty array results in default sorting.", "type": "array", "items": { "type": "string", diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 3e51c8589b..177d2828c0 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -525,8 +525,8 @@ They can be configured through the settings file 'winget settings'. {Locked="{0}"} Error message displayed when the user provides more than a single character command line alias argument after an alias argument specifier '-'. {0} is a placeholder replaced by the user's argument input. - Sort results by a specified field (can be repeated for multi-field sort) - Description for the --sort argument used to sort output by field name. Valid values are: name, id, version, source, available. + Sort results by a property (can be repeated) + Description for the --sort argument used to sort output by field name. Sort results in ascending order diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index f15883be34..9afde5a119 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -505,46 +505,15 @@ namespace AppInstaller::Settings WINGET_VALIDATE_SIGNATURE(OutputSortOrder) { - static constexpr std::string_view s_sortField_relevance = "relevance"; - static constexpr std::string_view s_sortField_name = "name"; - static constexpr std::string_view s_sortField_id = "id"; - static constexpr std::string_view s_sortField_version = "version"; - static constexpr std::string_view s_sortField_source = "source"; - static constexpr std::string_view s_sortField_available = "available"; - std::vector fields; for (auto const& entry : value) { - std::string lowered = Utility::ToLower(entry); - - if (lowered == s_sortField_relevance) - { - fields.emplace_back(SortField::Relevance); - } - else if (lowered == s_sortField_name) - { - fields.emplace_back(SortField::Name); - } - else if (lowered == s_sortField_id) - { - fields.emplace_back(SortField::Id); - } - else if (lowered == s_sortField_version) - { - fields.emplace_back(SortField::Version); - } - else if (lowered == s_sortField_source) - { - fields.emplace_back(SortField::Source); - } - else if (lowered == s_sortField_available) - { - fields.emplace_back(SortField::Available); - } - else + auto field = ConvertToSortField(entry); + if (!field) { return {}; } + fields.emplace_back(field.value()); } return fields; }