For templating#3553. Make search results downloads column optional #33024
Conversation
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
|
@levimatheri , please fix the tests |
There was a problem hiding this comment.
I agree with @YuliiaKovalova, please fix the tests before we can merge.
| @@ -153,7 +153,7 @@ private static string EvaluatePackageToShow(IReadOnlyList<SearchResult> searchRe | |||
| .DefineColumn(r => r.TemplateGroupInfo.Classifications, LocalizableStrings.ColumnNameTags, TabularOutputSettings.ColumnNames.Tags, defaultColumn: false, shrinkIfNeeded: true, minWidth: 10) | |||
| .DefineColumn(r => GetPackageInfo(r.PackageName, r.PackageOwners), out object? packageColumn, LocalizableStrings.ColumnNamePackageNameAndOwners, showAlways: true) | |||
| .DefineColumn(r => GetReservedMark(r.Reserved), LocalizableStrings.ColumnNameTrusted, showAlways: true, textAlign: TextAlign.Center) | |||
| .DefineColumn(r => r.PrintableTotalDownloads, out object? downloadsColumn, LocalizableStrings.ColumnNameTotalDownloads, showAlways: true, textAlign: TextAlign.Center); | |||
| .DefineColumn(r => r.PrintableTotalDownloads, LocalizableStrings.ColumnNameTotalDownloads, TabularOutputSettings.ColumnNames.Downloads, defaultColumn: false, textAlign: TextAlign.Center); | |||
There was a problem hiding this comment.
| .DefineColumn(r => r.PrintableTotalDownloads, LocalizableStrings.ColumnNameTotalDownloads, TabularOutputSettings.ColumnNames.Downloads, defaultColumn: false, textAlign: TextAlign.Center); | |
| .DefineColumn(r => r.PrintableTotalDownloads, LocalizableStrings.ColumnNameTotalDownloads, TabularOutputSettings.ColumnNames.Downloads, textAlign: TextAlign.Center); |
please keep this column available by default
| @@ -137,7 +137,8 @@ internal static Option<string[]> CreateColumnsOption() | |||
| TabularOutputSettings.ColumnNames.Author, | |||
| TabularOutputSettings.ColumnNames.Language, | |||
| TabularOutputSettings.ColumnNames.Type, | |||
| TabularOutputSettings.ColumnNames.Tags); | |||
| TabularOutputSettings.ColumnNames.Tags, | |||
There was a problem hiding this comment.
this option is now reused for dotnet new list and dotnet new search. Downloads are only applicable for dotnet new search, therefore please create two separate options now.
There was a problem hiding this comment.
Do you want these two separate options created in this same SharedOptionsFactory? Wasn't sure if having them both here defeats the purpose of a 'shared' class(?)
There was a problem hiding this comment.
imo, they can now live in respective command implementations
There was a problem hiding this comment.
@vlada-shubina It's getting a bit hairy with the NewCommand.Legacy which calls into SharedOptionsFactory for it's ColumnOptions, and the legacy list and search referencing the same ParentCommand. I tried to use AcceptOnlyFromAmong, but that seems to clear the allowed options, such that when I try dotnet new console --columns downloads --search, I get Argument 'downloads' not regonized. Must be one of: 'author' 'language' 'type' 'tags', because the LegacyList options overwrites the LegacySearch options since they are both using the same reference of Options from ParentCommand. Any suggestions on how to go about this?
levimatheri
force-pushed
the
templating-issue-3553/make-downloads-optional
branch
from
c97c8c5
to
d014fcd
Compare
|
working on fixing the tests |
For dotnet/templating issue 3553. Make search results downloads column optional. FYI @vlada-shubina
fixes dotnet/templating#3553