Skip to content
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

Add dotnet package search #36289

Merged
merged 13 commits into from
Jan 10, 2024
Merged

Add dotnet package search #36289

merged 13 commits into from
Jan 10, 2024

Conversation

Nigusu-Allehu
Copy link
Member

@Nigusu-Allehu Nigusu-Allehu commented Oct 19, 2023

Fixes: NuGet/Home#6060

Description

image

@danmoseley
Copy link
Member

Will this do all that Nuget.exe list does? (Which got some reason is always slow for me)

@Nigusu-Allehu
Copy link
Member Author

Nigusu-Allehu commented Oct 23, 2023

Will this do all that Nuget.exe list does? (Which got some reason is always slow for me)

Yes, it will, but there is some difference. The equivalent command would be dotnet package search --source <My source>, a search with no search term. This would list packages in My source, similar to nuget.exe list. However, it will only list the first 20 packages. This number can be changed by using the --skip and --take optons. For example, dotnet package search --source <My source> --take 1000 would list the first 1000. nuget.exe list on the other hand constantly contacts a source and output the packages in it.

About the speed, I will have to do some testing and compare the two to make a conclusion.

@Nigusu-Allehu Nigusu-Allehu marked this pull request as ready for review October 24, 2023 02:35
@danmoseley
Copy link
Member

@Nigusu-Allehu thanks

About the speed, I will have to do some testing and compare the two to make a conclusion.

The key here may be searching all the sources in parallel. I'm guessing that "restore" does not do this, because there is an ordering of feeds. But for listing, we want results from all the feeds.


namespace Microsoft.DotNet.Cli
{
internal class PackageSearchCommand : CommandBase
Copy link
Contributor

@WeihanLi WeihanLi Nov 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe could be sealed

Suggested change
internal class PackageSearchCommand : CommandBase
internal sealed class PackageSearchCommand : CommandBase

@marcpopMSFT
Copy link
Member

I don't see any tests for the new functionality.

Copy link

@JonDouglas JonDouglas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great! amazing job.

there are a couple of thoughts I have about aliasing but they dont necessarily need to be addressed here.

dotnet search package <> dotnet package search <> dotnet search even.

@baronfel thoughts on above? Maybe in the far future we can refactor a completely 3-in-1 dotnet search experience where tools/workloads/packages all search nicely!

@baronfel
Copy link
Member

baronfel commented Jan 9, 2024

I'm kinda sad that we have dotnet package search given that previous commands for interacting with packages are verb-forward instead of noun-forward. This is already a large source of user confusion, and I don't expect this to help :(

If we now have a package root noun, I'd really like to consider adding list and add subcommands to it and beginning the process of deprecating the old commands as part of .NET 9.

Having said that, awesome to have native package search in the CLI :D 🎆

@JonDouglas
Copy link

@baronfel Yes, a bit sad but at the same time these are challenges to work through. The prior art may have inspired the names too much i.e. workloads/tool, and hopefully a quick alias can fix that and even lead to some delight of users who are dyslexic like me accidentally using this command! Otherwise if this is a major concern by SDK, we should refactor and then alias anyway given it could be helpful.


namespace Microsoft.DotNet.Cli
{
internal class PackageCommandParser
Copy link
Contributor

@WeihanLi WeihanLi Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe could be static since there're no instance members

Suggested change
internal class PackageCommandParser
internal static class PackageCommandParser

Co-authored-by: Weihan Li <weihanli@outlook.com>
@Nigusu-Allehu Nigusu-Allehu merged commit 1d10b7c into main Jan 10, 2024
16 checks passed
@Nigusu-Allehu Nigusu-Allehu deleted the nyenework/package-search branch January 10, 2024 23:04

public static CliCommand GetCommand()
{
CliCommand command = new DocumentedCommand("package", DocsLink);
Copy link
Contributor

@kartheekp-ms kartheekp-ms Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var command = new DocumentedCommand("restore", DocsLink, LocalizableStrings.AppFullName);

RestoreCommandParser passes additional argument (LocalizableStrings.AppFullName). I am not sure on what is the behavior change because of passing the additional parameter.

@Nigusu-Allehu
Copy link
Member Author

/backport to release/8.0.2xx

Copy link
Contributor

Started backporting to release/8.0.2xx: https://github.com/dotnet/sdk/actions/runs/7482205215

Copy link
Contributor

@Nigusu-Allehu an error occurred while backporting to release/8.0.2xx, please check the run log for details!

Error: @Nigusu-Allehu is not a repo collaborator, backporting is not allowed. If you're a collaborator please make sure your dotnet team membership visibility is set to Public on https://github.com/orgs/dotnet/people?query=Nigusu-Allehu

@marcpopMSFT
Copy link
Member

/backport to release/8.0.2xx

Copy link
Contributor

Started backporting to release/8.0.2xx: https://github.com/dotnet/sdk/actions/runs/7482215172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for dotnet search command (equivalent to nuget.exe list, later search)
9 participants