-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Implement sort logic for winget list output #6191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
49ae654
a210423
918fe25
7469608
6f7773f
813c8af
a32a676
46a5707
dc65788
ea81abd
0d86ce9
aaf8e9c
8ac341d
c19ce84
4065b64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -290,6 +290,7 @@ LEBOM | |
| lhs | ||
| LIBYAML | ||
| liv | ||
| listsort | ||
| liwpx | ||
| localizationpriority | ||
| localsource | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| #include "Workflows/CompletionFlow.h" | ||
| #include "Workflows/WorkflowBase.h" | ||
| #include "Resources.h" | ||
| #include <AppInstallerLanguageUtilities.h> | ||
|
|
||
| namespace AppInstaller::CLI | ||
| { | ||
|
|
@@ -32,7 +33,7 @@ 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::Sort, Resource::String::SortArgumentDescription, ArgumentType::Standard }.SetCountLimit(AppInstaller::GetAllExponentialEnumValues<Settings::SortField>(Settings::SortField::None).size()), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: There is probably a more efficient way that we could do this at compile time, like |
||
| Argument{ Execution::Args::Type::SortAscending, Resource::String::SortAscendingArgumentDescription, ArgumentType::Flag }, | ||
| Argument{ Execution::Args::Type::SortDescending, Resource::String::SortDescendingArgumentDescription, ArgumentType::Flag }, | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,192 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
| #include "pch.h" | ||
| #include "PackageTableSortHelper.h" | ||
|
|
||
| namespace AppInstaller::CLI::Workflow | ||
| { | ||
| using namespace Settings; | ||
|
|
||
| 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, | ||
| SortField fieldMask) | ||
| : OriginalIndex(originalIndex) | ||
| { | ||
| if (WI_IsFlagSet(fieldMask, SortField::Name)) | ||
| { | ||
| FoldedName = Utility::FoldCase(name); | ||
| } | ||
| if (WI_IsFlagSet(fieldMask, SortField::Id)) | ||
| { | ||
| FoldedId = Utility::FoldCase(id); | ||
| } | ||
| if (WI_IsFlagSet(fieldMask, SortField::Source)) | ||
| { | ||
| FoldedSource = Utility::FoldCase(source); | ||
| } | ||
| if (WI_IsFlagSet(fieldMask, SortField::Version)) | ||
| { | ||
| ParsedInstalledVersion = Utility::Version{ std::string{ installedVersion } }; | ||
| } | ||
| if (WI_IsFlagSet(fieldMask, SortField::Available)) | ||
| { | ||
| if (!availableVersion.empty()) | ||
| { | ||
| ParsedAvailableVersion = Utility::Version{ std::string{ availableVersion } }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| SortField ComputeSortFieldMask(const std::vector<SortField>& sortFields) | ||
| { | ||
| SortField mask = SortField::None; | ||
| for (const auto& f : sortFields) | ||
| { | ||
| mask |= f; | ||
| } | ||
| return mask; | ||
| } | ||
|
|
||
| SortParameters ResolveSortParameters( | ||
| const std::vector<std::string_view>& explicitSortArgs, | ||
| bool hasQuery, | ||
| bool hasExplicitAscending, | ||
| bool hasExplicitDescending) | ||
| { | ||
| SortParameters result; | ||
| std::vector<SortField> sortFields; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: why not just use If the answer is allowing |
||
|
|
||
| if (!explicitSortArgs.empty()) | ||
| { | ||
| for (const auto& arg : explicitSortArgs) | ||
| { | ||
| auto field = ConvertToSortField(arg); | ||
| WI_ASSERT(field.has_value()); | ||
| if (field.has_value()) | ||
| { | ||
| sortFields.emplace_back(field.value()); | ||
| } | ||
| } | ||
| } | ||
| else | ||
| { | ||
| sortFields = User().Get<Setting::OutputSortOrder>(); | ||
|
|
||
| if (sortFields.empty()) | ||
| { | ||
| if (hasQuery) | ||
| { | ||
| // Preserve relevance ordering when a free-text query is present | ||
| // and no explicit sort preference is configured. | ||
| return result; // ShouldSort = false | ||
| } | ||
|
|
||
| // No settings, no query — default to name sort. | ||
| sortFields.emplace_back(SortField::Name); | ||
| } | ||
| } | ||
|
|
||
| // Relevance-only means preserve source ordering. | ||
| if (sortFields.size() == 1 && sortFields[0] == SortField::Relevance) | ||
| { | ||
| return result; // ShouldSort = false | ||
| } | ||
|
|
||
| // Resolve direction | ||
| SortDirection direction = SortDirection::Ascending; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: same here For both of these it is more a style/preference thing as performance should be equivalent. I just personally like removing the 4 lines of code but I'm open to hearing about reasons not to. I can imagine a reason that is about first evaluating and then collecting the data independently for clarity. I can also imagine a NRVO type argument (although maybe this should be the constructor of |
||
| if (hasExplicitDescending) | ||
| { | ||
| direction = SortDirection::Descending; | ||
| } | ||
| else if (hasExplicitAscending) | ||
| { | ||
| direction = SortDirection::Ascending; | ||
| } | ||
| else | ||
| { | ||
| direction = User().Get<Setting::OutputSortDirection>(); | ||
| } | ||
|
|
||
| result.ShouldSort = true; | ||
| result.Fields = std::move(sortFields); | ||
| result.Direction = direction; | ||
| return result; | ||
| } | ||
|
|
||
| int CompareByField(const SortablePackageEntry& a, const SortablePackageEntry& b, SortField field) | ||
| { | ||
| 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: | ||
| { | ||
| // std::optional comparison: nullopt < any value. | ||
| // We want has-version to sort before no-version in ascending order, | ||
| // so reverse the comparison operands. | ||
| if (a.ParsedAvailableVersion.has_value() != b.ParsedAvailableVersion.has_value()) | ||
| { | ||
| return a.ParsedAvailableVersion.has_value() ? -1 : 1; | ||
| } | ||
| if (a.ParsedAvailableVersion.has_value() && b.ParsedAvailableVersion.has_value()) | ||
| { | ||
| if (a.ParsedAvailableVersion.value() < b.ParsedAvailableVersion.value()) return -1; | ||
| if (b.ParsedAvailableVersion.value() < a.ParsedAvailableVersion.value()) return 1; | ||
| } | ||
| return 0; | ||
| } | ||
| case SortField::Relevance: | ||
| // Relevance has no precomputed sort key — preserve original ordering. | ||
| return 0; | ||
| default: | ||
| WI_ASSERT(false); | ||
| return 0; | ||
| } | ||
| } | ||
|
|
||
| void SortEntries( | ||
| std::vector<SortablePackageEntry>& entries, | ||
| const std::vector<SortField>& sortFields, | ||
| SortDirection direction) | ||
|
Comment on lines
+164
to
+165
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| { | ||
| if (entries.size() <= 1 || sortFields.empty()) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Relevance-only means no sorting | ||
| if (sortFields.size() == 1 && sortFields[0] == SortField::Relevance) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| { | ||
| 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 == SortDirection::Ascending ? (cmp < 0) : (cmp > 0); | ||
| } | ||
| } | ||
| return false; | ||
| }); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.