-
Notifications
You must be signed in to change notification settings - Fork 659
Fix TFM ordering in Dependencies tab using NuGetFrameworkSorter #10655
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: dev
Are you sure you want to change the base?
Changes from 6 commits
c1bab61
934baa8
434a643
70ce314
5c08fda
299e3ce
7a1b3c0
d85037b
d86e79d
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 |
|---|---|---|
|
|
@@ -24,21 +24,41 @@ public DependencySetsViewModel(IEnumerable<PackageDependency> packageDependencie | |
|
|
||
| OnlyHasAllFrameworks = dependencySets.Count() == 1 && dependencySets.First().Key == null; | ||
|
|
||
| // Create a list to hold TFM string, parsed framework, and dependencies for proper sorting | ||
| var frameworkGroups = new List<(string tfmString, NuGetFramework framework, string friendlyName, IEnumerable<DependencyViewModel> dependencies)>(); | ||
|
|
||
| foreach (var dependencySet in dependencySets) | ||
| { | ||
| var targetFramework = dependencySet.Key == null | ||
| ? "All Frameworks" | ||
| : NuGetFramework.Parse(dependencySet.Key).ToFriendlyName(); | ||
| string tfmString = dependencySet.Key; | ||
| string friendlyName; | ||
| NuGetFramework framework; | ||
|
|
||
| if (!DependencySets.ContainsKey(targetFramework)) | ||
| if (tfmString == null) | ||
| { | ||
| friendlyName = "All Frameworks"; | ||
| framework = null; | ||
| } | ||
| else | ||
| { | ||
| DependencySets.Add(targetFramework, | ||
| dependencySet.OrderBy(x => x.Id).Select(d => d.Id == null ? null : new DependencyViewModel(d.Id, d.VersionSpec))); | ||
| framework = NuGetFramework.Parse(tfmString); | ||
| friendlyName = framework.ToFriendlyName(); | ||
| } | ||
|
|
||
| var dependencies = dependencySet.OrderBy(x => x.Id, StringComparer.OrdinalIgnoreCase).Select(d => d.Id == null ? null : new DependencyViewModel(d.Id, d.VersionSpec)); | ||
| frameworkGroups.Add((tfmString, framework, friendlyName, dependencies)); | ||
| } | ||
|
|
||
| // Order the top level frameworks by their resulting friendly name | ||
| DependencySets = DependencySets.OrderBy(x => x.Key).ToDictionary(x => x.Key, x => x.Value); | ||
| // Sort by framework using NuGetFrameworkSorter, with null frameworks (All Frameworks) first | ||
| var sortedGroups = frameworkGroups.OrderBy(g => g.framework, NullableFrameworkComparer.Instance); | ||
|
|
||
| // Build the final dictionary with friendly names | ||
| foreach (var group in sortedGroups) | ||
| { | ||
| if (!DependencySets.ContainsKey(group.friendlyName)) | ||
| { | ||
| DependencySets.Add(group.friendlyName, group.dependencies); | ||
|
Contributor
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. I'm very confused how this works. Why does
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. I think you need to address (at)copilot to get a response.
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. @copilot - can you check this comment thread please?
Contributor
Author
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. You're correct that relying on Dictionary insertion order was problematic, especially for .NET Framework 4.7.2 where it's not guaranteed. Changed |
||
| } | ||
| } | ||
| } | ||
| catch (Exception e) | ||
| { | ||
|
|
@@ -72,5 +92,20 @@ public DependencyViewModel(string id, string versionSpec) | |
| public string VersionSpec { get; private set; } | ||
| public string PackageUrl { get; private set; } | ||
| } | ||
|
|
||
| private class NullableFrameworkComparer : IComparer<NuGetFramework> | ||
| { | ||
|
joelverhagen marked this conversation as resolved.
|
||
| public static readonly NullableFrameworkComparer Instance = new NullableFrameworkComparer(); | ||
|
|
||
| public int Compare(NuGetFramework x, NuGetFramework y) | ||
| { | ||
| // Put "All Frameworks" (null) first | ||
| if (x == null && y == null) return 0; | ||
| if (x == null) return -1; | ||
| if (y == null) return 1; | ||
| // Use NuGetFrameworkSorter for actual frameworks | ||
| return NuGetFrameworkSorter.Instance.Compare(x, y); | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string.IsNullOrEmpty(tfmString )