Implement sort logic for winget list output #6191
Implement sort logic for winget list output
#6191Madhusudhan-MSFT wants to merge 13 commits intomicrosoft:masterfrom
Conversation
… add comprehensive tests
…pp file ## Summary Every other Workflow header in the project follows the declarations-only convention with a matching .cpp file. ListSortHelper.h was the only outlier using inline implementations. This refactor aligns it with the codebase convention and eliminates the duplicated sort loop between ListSortHelper.h and WorkflowBase.cpp. ## Changes - **ListSortHelper.h → declarations only**: Remove inline implementations of CompareByField and SortEntries, keeping only the struct definition and function signatures - **ListSortHelper.cpp**: New compilation unit with the extracted implementations of CompareByField and SortEntries - **WorkflowBase.cpp sort integration**: Replace the duplicated inline sort loop with an index-based permutation sort that projects InstalledPackagesTableLine into SortablePackageEntry and delegates field comparison to the shared CompareByField - Remove the local CompareByField adapter that was creating temporary SortablePackageEntry copies per comparison call - Align include path and grouping with repo conventions (bare sibling name, local before system) - Simplify GetArgs access to match established MultiQueryFlow pattern ## Impact - Sort logic now lives in a single shared location, reusable by future commands (search, upgrade, font list) without duplication - Follows the established Workflows/ header convention (24/24 other headers use .h + .cpp) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add --sort, --ascending, and --descending options to the options table. Add new 'Sorting output' section documenting: - CLI sort arguments with multi-field examples - Settings-based sortOrder configuration - Resolution order (CLI > settings > default) - Relevance protection behavior for queries
Validate --sort argument values in Command::ValidateArguments following the same pattern as --scope and --authentication-mode validation. Invalid values produce a clear error message listing valid options (name, id, version, source, available, relevance). Centralized in Command.cpp so any future command adding --sort gets validation automatically.
This comment has been minimized.
This comment has been minimized.
|
Is this only applicable to |
This seems like it would be counter to what a user would want. If I did |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The current implementation is scoped to |
This was discussed and agreed upon during the initial design. The intent is to preserve source relevance ordering for queries since a settings-based sort could push the best match down without the user realizing why. Explicit @denelon — I believe this aligns with what we discussed during the design phase, would you mind confirming? |
I think there is, but agree with the approach of limiting the scope for now and letting Demitrius figure out when to bring the rest in |
From the design portion, my feedback was that the default behavior should be to remain unchanged with a query but could have a new ordering without a query. If the user does supply a setting value, my expectation would be that it would be used with or without a query unless overridden with a command line argument. What constitutes a query? Probably only the actual |
…edback - Remove HasRelevanceAffectingArgs function (overly broad filter) - Narrow relevance preservation to only the free-text query argument - Settings output.sortOrder now applies even when query is present (user explicitly configured sort preference takes priority) - Filter args (--id, --name, --tag etc.) no longer block settings sort since they use exact/substring matching with no meaningful relevance - Update list.md documentation to reflect new behavior - Resolution: CLI --sort > settings (always) > default relevance (query only)
Thanks for the clarification, John. Updated in
Removed |
- Fix comment: 'enable' -> 'ease' unit testing (not impossible, just decoupled) - Remove stale comment from deleted HasRelevanceAffectingArgs function - Add THROW_HR(E_UNEXPECTED) assertion for invalid sort values that bypass validation - Change --sort count limit from 7 to 6 (matches actual SortField enum count)
When no sort preferences are configured (no CLI --sort, no settings) and no positional query is present, default to sorting by name ascending for deterministic, user-friendly output. With a positional query, relevance ordering is preserved unless the user explicitly configures sort preferences. Update list.md to document this conditional default behavior.
Restructure ListSortHelper to own the production sort path: - SortablePackageEntry now precomputes case-folded strings and parsed versions at construction time, avoiding repeated FoldCase/Version parsing during comparisons. - Add OriginalIndex field so entries track their source position. - Add SortBy<T> template that converts arbitrary item vectors into SortablePackageEntry projections, sorts, then reorders the source vector to match. This is the production code path. - WorkflowBase calls SortBy with a conversion lambda, replacing the inline permutation sort. - SortEntries is no longer test-only — it is the same code path that production uses via SortBy. - Add SortBy template tests to validate the end-to-end pipeline with custom source types.
Rename to reflect broader scope — the helper sorts package table rows for any table-based command (list, search, upgrade), not just list.
Change SortField to flag-bit enum (uint32_t) and add DEFINE_ENUM_FLAG_OPERATORS so sort fields compose into a bitmask. SortBy computes the mask once and passes it through the converter to SortablePackageEntry's constructor, which now only precomputes fields that are actually needed for the requested sort.
- Add static_assert on SortField::Available at enum definition site to catch new values that bypass constructor handling. - Replace invalid-sort assertion with FAIL_FAST_MSG for clearer diagnostics. - Collapse two consecutive sortFields.empty() checks into a single branch. - Format SortablePackageEntry constructor arguments one-per-line. - Add --query example to list.md Example queries section.
d92410d to
5aae88f
Compare
Summary
Implements the sort logic for
winget listoutput, completing the feature requested in #4238. Builds on PR #6177 (settings parsing + CLI handling).Changes
Sort orchestration (
WorkflowBase.cpp):SortInstalledPackagesTableLines— resolves sort fields from CLI--sort> settingsoutput.sortOrder> default (sort by name), determines direction, sorts viastd::stable_sort, applies permutation back.-q) is used and no sort preference is configured, relevance ordering is preserved. Ifoutput.sortOrderis configured, it is respected even with queries.FAIL_FAST_MSGfor unrecognized sort field values — ensures invalid enum values crash loudly in debug builds.Sort helper (
PackageTableSortHelper.h/.cpp):SortablePackageEntry— holds sortable fields, computed only for fields needed viaComputeSortFieldMaskbitmask optimization.CompareByField— case-insensitive ordinal comparison viaFoldCase.SortBy— template sort executor used by the workflow;SortEntries— standalone wrapper for unit tests.SortFieldenum uses flag-bit values (uint32_t) withDEFINE_ENUM_FLAG_OPERATORSfor efficient bitmask field tracking.CLI integration (
ListCommand.cpp):--sort(repeatable, limit 6),--ascending/--asc,--descending/--descarguments.Argument validation (
Command.cpp):--sortvalues inValidateArguments, following the--scope/--authentication-modepattern.Documentation (
list.md):--sort,--ascending,--descendingin options table. New "Sorting output" section.Design decisions
-q) without configured sort preserves source relevance ranking. Settings override this if configured — explicit user preference takes priority.stable_sort--sort source --sort namegroups by source, alphabetical within).SortFieldenumComputeSortFieldMaskto skip unused field extraction — measurable win for large package lists.FAIL_FAST_MSGon invalid sortHow validated
Unit tests — 20 test cases:
PackageTableSortHelper.cpp: single/multi-field sorting, both directions, edge cases, case-insensitive comparison, relevance no-opCommand.cpp: readsListCommand::GetArguments()limit and validates against knownConvertToSortFieldstrings — breaks if enum changes without updating limitCommand.cpp: verifiesSetCountLimit(6)rejects 7--sortvalues withTooManyArgErrorManual testing — 21 scenarios on
wingetdev(Debug x64): defaults, single/multi-field sorts, direction flags, query + sort interaction, settings precedence, edge cases, invalid input. All passing.Fixes #4238