Implement sort logic for winget list output #6191
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -290,6 +290,7 @@ LEBOM | |
| lhs | ||
| LIBYAML | ||
| liv | ||
| listsort | ||
| liwpx | ||
| localizationpriority | ||
| localsource | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
| #include "pch.h" | ||
| #include "PackageTableSortHelper.h" | ||
|
|
||
| namespace AppInstaller::CLI::Workflow | ||
| { | ||
| SortablePackageEntry::SortablePackageEntry( | ||
| size_t originalIndex, | ||
| std::string_view name, | ||
| std::string_view id, | ||
| std::string_view installedVersion, | ||
| std::string_view availableVersion, | ||
| std::string_view source, | ||
| Settings::SortField fieldMask) | ||
| : OriginalIndex(originalIndex) | ||
| { | ||
| if (WI_IsFlagSet(fieldMask, Settings::SortField::Name)) | ||
| { | ||
| FoldedName = Utility::FoldCase(name); | ||
| } | ||
| if (WI_IsFlagSet(fieldMask, Settings::SortField::Id)) | ||
| { | ||
| FoldedId = Utility::FoldCase(id); | ||
| } | ||
| if (WI_IsFlagSet(fieldMask, Settings::SortField::Source)) | ||
| { | ||
| FoldedSource = Utility::FoldCase(source); | ||
| } | ||
| if (WI_IsFlagSet(fieldMask, Settings::SortField::Version)) | ||
| { | ||
| ParsedInstalledVersion = Utility::Version{ std::string{ installedVersion } }; | ||
| } | ||
| if (WI_IsFlagSet(fieldMask, Settings::SortField::Available)) | ||
| { | ||
| HasAvailableVersion = !availableVersion.empty(); | ||
| if (HasAvailableVersion) | ||
| { | ||
| ParsedAvailableVersion = Utility::Version{ std::string{ availableVersion } }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Settings::SortField ComputeSortFieldMask(const std::vector<Settings::SortField>& sortFields) | ||
| { | ||
| Settings::SortField mask = Settings::SortField::Relevance; | ||
|
Member
There was a problem hiding this comment. Why is |
||
| for (const auto& f : sortFields) | ||
| { | ||
| mask |= f; | ||
| } | ||
| return mask; | ||
| } | ||
|
|
||
| int CompareByField(const SortablePackageEntry& a, const SortablePackageEntry& b, Settings::SortField field) | ||
| { | ||
| using Settings::SortField; | ||
|
|
||
| switch (field) | ||
| { | ||
| case SortField::Name: | ||
| return a.FoldedName.compare(b.FoldedName); | ||
| case SortField::Id: | ||
| return a.FoldedId.compare(b.FoldedId); | ||
| case SortField::Version: | ||
| { | ||
| if (a.ParsedInstalledVersion < b.ParsedInstalledVersion) return -1; | ||
| if (b.ParsedInstalledVersion < a.ParsedInstalledVersion) return 1; | ||
| return 0; | ||
| } | ||
| case SortField::Source: | ||
| return a.FoldedSource.compare(b.FoldedSource); | ||
| case SortField::Available: | ||
| { | ||
| if (a.HasAvailableVersion != b.HasAvailableVersion) | ||
|
Member
There was a problem hiding this comment. Converting to an optional gets automatic empty vs not-empty comparison along with the value comparison. Look it up to ensure it does what you want though, as it might put the empties at the wrong end from where you want. |
||
| { | ||
| // Ascending: has-update sorts before no-update | ||
| return a.HasAvailableVersion ? -1 : 1; | ||
| } | ||
| if (a.HasAvailableVersion && b.HasAvailableVersion) | ||
| { | ||
| if (a.ParsedAvailableVersion < b.ParsedAvailableVersion) return -1; | ||
| if (b.ParsedAvailableVersion < a.ParsedAvailableVersion) return 1; | ||
| } | ||
| return 0; | ||
| } | ||
| default: | ||
| return 0; | ||
| } | ||
| } | ||
|
|
||
| void SortEntries( | ||
| std::vector<SortablePackageEntry>& entries, | ||
| const std::vector<Settings::SortField>& sortFields, | ||
| Settings::SortDirection direction) | ||
| { | ||
| if (entries.size() <= 1 || sortFields.empty()) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Relevance-only means no sorting | ||
| if (sortFields.size() == 1 && sortFields[0] == Settings::SortField::Relevance) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| std::stable_sort(entries.begin(), entries.end(), | ||
| [&sortFields, direction](const SortablePackageEntry& a, const SortablePackageEntry& b) | ||
| { | ||
| for (const auto& field : sortFields) | ||
| { | ||
| int cmp = CompareByField(a, b, field); | ||
| if (cmp != 0) | ||
| { | ||
| return direction == Settings::SortDirection::Ascending ? (cmp < 0) : (cmp > 0); | ||
| } | ||
| } | ||
| return false; | ||
| }); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,95 @@ | ||||||||
| // Copyright (c) Microsoft Corporation. | ||||||||
| // Licensed under the MIT License. | ||||||||
| #pragma once | ||||||||
| #include <AppInstallerStrings.h> | ||||||||
| #include <AppInstallerVersions.h> | ||||||||
| #include <winget/UserSettings.h> | ||||||||
|
|
||||||||
| #include <algorithm> | ||||||||
| #include <vector> | ||||||||
|
|
||||||||
| namespace AppInstaller::CLI::Workflow | ||||||||
| { | ||||||||
| // Lightweight sortable representation of a package row with precomputed sort keys. | ||||||||
| // Decoupled from ICompositePackage/IPackageVersion to ease unit testing. | ||||||||
| struct SortablePackageEntry | ||||||||
| { | ||||||||
| size_t OriginalIndex = 0; | ||||||||
|
|
||||||||
| // Precomputed case-folded sort keys | ||||||||
| std::string FoldedName; | ||||||||
| std::string FoldedId; | ||||||||
| std::string FoldedSource; | ||||||||
|
|
||||||||
| // Precomputed parsed versions | ||||||||
| Utility::Version ParsedInstalledVersion; | ||||||||
| Utility::Version ParsedAvailableVersion; | ||||||||
| bool HasAvailableVersion = false; | ||||||||
|
Comment on lines
+26
to
+27
Member
There was a problem hiding this comment.
Suggested change
|
||||||||
|
|
||||||||
| SortablePackageEntry() = default; | ||||||||
|
|
||||||||
| SortablePackageEntry( | ||||||||
| size_t originalIndex, | ||||||||
| std::string_view name, | ||||||||
| std::string_view id, | ||||||||
| std::string_view installedVersion, | ||||||||
| std::string_view availableVersion, | ||||||||
| std::string_view source, | ||||||||
| Settings::SortField fieldMask); | ||||||||
| }; | ||||||||
|
|
||||||||
| // Compares two sortable entries by the given field using precomputed sort keys. | ||||||||
| // Returns negative if a < b, positive if a > b, 0 if equal. | ||||||||
| int CompareByField(const SortablePackageEntry& a, const SortablePackageEntry& b, Settings::SortField field); | ||||||||
|
|
||||||||
| // Sorts a vector of sortable entries by the given fields and direction. | ||||||||
| void SortEntries( | ||||||||
| std::vector<SortablePackageEntry>& entries, | ||||||||
| const std::vector<Settings::SortField>& sortFields, | ||||||||
| Settings::SortDirection direction); | ||||||||
|
|
||||||||
| // Computes a bitmask of all sort fields so the constructor can skip unused fields. | ||||||||
| Settings::SortField ComputeSortFieldMask(const std::vector<Settings::SortField>& sortFields); | ||||||||
|
|
||||||||
| // Sorts a vector of arbitrary items by projecting each into a SortablePackageEntry | ||||||||
| // via a caller-supplied converter, sorting the projections, then reordering the | ||||||||
| // source vector to match. The converter signature is: | ||||||||
| // SortablePackageEntry converter(const T& item, size_t index) | ||||||||
| // The caller is responsible for pre-computing the SortFieldMask and capturing | ||||||||
| // it in the converter closure so the SortablePackageEntry constructor only | ||||||||
| // initializes the fields actually needed for comparison. | ||||||||
| template <typename T, typename Converter> | ||||||||
| void SortBy( | ||||||||
| std::vector<T>& items, | ||||||||
| Converter&& converter, | ||||||||
| const std::vector<Settings::SortField>& sortFields, | ||||||||
| Settings::SortDirection direction) | ||||||||
| { | ||||||||
| if (items.size() <= 1 || sortFields.empty()) | ||||||||
| { | ||||||||
| return; | ||||||||
| } | ||||||||
|
|
||||||||
| if (sortFields.size() == 1 && sortFields[0] == Settings::SortField::Relevance) | ||||||||
| { | ||||||||
| return; | ||||||||
| } | ||||||||
|
|
||||||||
| std::vector<SortablePackageEntry> entries; | ||||||||
| entries.reserve(items.size()); | ||||||||
| for (size_t i = 0; i < items.size(); ++i) | ||||||||
| { | ||||||||
| entries.push_back(converter(items[i], i)); | ||||||||
| } | ||||||||
|
|
||||||||
| SortEntries(entries, sortFields, direction); | ||||||||
|
|
||||||||
| std::vector<T> sorted; | ||||||||
| sorted.reserve(items.size()); | ||||||||
| for (const auto& entry : entries) | ||||||||
| { | ||||||||
| sorted.push_back(std::move(items[entry.OriginalIndex])); | ||||||||
| } | ||||||||
| items = std::move(sorted); | ||||||||
|
Comment on lines
+78
to
+93
Member
There was a problem hiding this comment. Why not sort I would also think that having a case-insensitive comparison function could be way better in many cases. For example, if the first letters of each string are all different, we would end up only folding those, instead of the full strings.
Contributor
Author
There was a problem hiding this comment. Pre-fold was requested by JohnMcPMS in an earlier review round. At this scale (~100-200 items) the difference is negligible. Happy to revisit if profiling shows otherwise.
Member
There was a problem hiding this comment. At 200 items, a merge sort would expect 2125 comparisons ( I don't expect that we will have this many items regularly, but at the small numbers that are likely with a query I don't think any choice matters much. I would rather optimize for the worst case even if the best case gets a little worse. |
||||||||
| } | ||||||||
| } | ||||||||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.