Skip to content

[Settings, ListCommand] Add sort types, settings infrastructure, and CLI arguments for output ordering#6177

Open
Madhusudhan-MSFT wants to merge 6 commits intomicrosoft:masterfrom
Madhusudhan-MSFT:user/masudars/list-sort-cli-args
Open

[Settings, ListCommand] Add sort types, settings infrastructure, and CLI arguments for output ordering#6177
Madhusudhan-MSFT wants to merge 6 commits intomicrosoft:masterfrom
Madhusudhan-MSFT:user/masudars/list-sort-cli-args

Conversation

@Madhusudhan-MSFT
Copy link
Copy Markdown
Contributor

@Madhusudhan-MSFT Madhusudhan-MSFT commented Apr 25, 2026

Summary

Add settings infrastructure and CLI arguments for configurable output sorting in
winget list. This lays the full type system, settings plumbing, and argument
registration without changing any runtime behavior — sort logic is wired in a
follow-up PR.

Part of #4238

Changes

Settings types and schema

  • SortField and SortDirection enums: Add SortField (Relevance, Name, Id, Version,
    Source, Available) and SortDirection (Ascending, Descending) to the settings type
    system. Relevance preserves the current natural ordering and is publicly configurable
    (users can set ["relevance"] to explicitly opt out of sorting).
  • Settings entries: Register OutputSortOrder and OutputSortDirection with typed
    validation using lowercase-once comparison (Utility::ToLower + ==). Default sort
    order is empty ({}) — an empty vector acts as a sentinel meaning "use context-aware
    defaults" (the follow-up PR's sort logic decides: relevance for queries, Name for
    no-query).
  • JSON schema: Extend settings.schema.0.2.json with an output section containing
    sortOrder (string array with relevance, name, id, version, source, available) and
    sortDirection (string enum with ascending default).
  • Static constexpr pattern: String constants for field name and direction comparisons
    follow the established static constexpr std::string_view pattern used by existing
    validators.

CLI arguments

  • Execution args: Add Sort, SortAscending, and SortDescending arg types to
    ExecutionArgs.h.
  • Argument registration: Define --sort (multi-use, max 7 values), --ascending/--asc,
    and --descending/--desc in Argument.cpp with resource string descriptions.
    --ascending and --descending are mutually exclusive via ExclusiveOf.
  • ListCommand: Add the three new arguments to ListCommand::GetArguments().
  • Resource strings: Add localized descriptions for all three arguments in
    winget.resw and resource IDs in Resources.h.

Impact

  • No behavioral change — settings and arguments are defined but not yet consumed by
    any workflow. Existing winget list output remains unchanged.
  • winget list --sort name is accepted without error but output remains unsorted until
    the sort logic PR lands.
  • --ascending and --descending are mutually exclusive — passing both produces an error.

How Validated

  • Unit tests: 17 test sections across SettingOutputSortOrder and SettingOutputSortDirection
    covering default values (empty fallback), single/multi-field, case insensitivity, empty
    array, relevance field, duplicate fields, invalid fields, mixed valid/invalid, and wrong
    JSON types (68 assertions total).
  • Manual verification: wingetdev list --sort name accepted, wingetdev list --help
    shows new arguments.

Follow-up

  • Sort logic wiring, query-aware defaults, and
    comprehensive sort tests will be in a follow-up PR.

…gs infrastructure

## 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)
…dators

Follow established codebase pattern (NetworkDownloader, LoggingLevel) of
declaring static constexpr std::string_view variables for string comparisons
instead of inline string literals.
@Madhusudhan-MSFT Madhusudhan-MSFT force-pushed the user/masudars/list-sort-cli-args branch from 314ebb6 to edeb3bb Compare April 27, 2026 17:00
@github-actions

This comment has been minimized.

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 microsoft#6176.
@Madhusudhan-MSFT Madhusudhan-MSFT force-pushed the user/masudars/list-sort-cli-args branch from edeb3bb to abebe97 Compare April 27, 2026 17:11
Copy link
Copy Markdown
Member

@JohnMcPMS JohnMcPMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This includes the previous change, and I don't think there is enough in them to justify a split PR (and they have some shared portions, like which values are valid).

Comment thread src/AppInstallerCommonCore/UserSettings.cpp Outdated
… 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
…guments

## 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)
@Madhusudhan-MSFT Madhusudhan-MSFT force-pushed the user/masudars/list-sort-cli-args branch from abebe97 to f15a3e1 Compare April 27, 2026 18:29
@Madhusudhan-MSFT
Copy link
Copy Markdown
Contributor Author

Thanks John — understood. I split them as stacked PRs for independent scoping, but agree the change here is small. The latest PR1 fixes (shared ConvertToSortField, empty-vector default, public relevance) have been rebased into this branch.

@Madhusudhan-MSFT Madhusudhan-MSFT marked this pull request as ready for review April 27, 2026 18:52
@Madhusudhan-MSFT Madhusudhan-MSFT requested a review from a team as a code owner April 27, 2026 18:52
@JohnMcPMS
Copy link
Copy Markdown
Member

I don't see a shared function here or in the other PR. Can we just close one of the PRs?

@Madhusudhan-MSFT Madhusudhan-MSFT changed the title [ListCommand, Args] Register --sort, --ascending, and --descending arguments [Settings, ListCommand] Add sort types, settings infrastructure, and CLI arguments for output ordering Apr 28, 2026
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>
@Madhusudhan-MSFT
Copy link
Copy Markdown
Contributor Author

Sorry for the confusion — ConvertToSortField was in my local changes, I forgot to publish it to this PR. I was planning to include it as part of the follow-up PR that wires in all the actual sort logic. I've included it now (commit 3d9c1e4). I've also closed the other PR (#6176) so everything is in a single PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants