-
Notifications
You must be signed in to change notification settings - Fork 1.1k
allow @ syntax for package versions in package add #47961
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
Conversation
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.
Pull Request Overview
This PR introduces support for the @ syntax to specify package versions in the dotnet package add command and prevents users from manually providing a --version option when a version is already embedded in the package string.
- Update Program.cs to use PackageIdentity instead of a simple string for handling packages and transform arguments.
- Enhance PackageAddCommandParser to include a custom parser for package identity extraction and a validator that disallows the --version option when a version is provided via the @ syntax.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/Cli/dotnet/Commands/Package/Add/Program.cs | Refactored to use PackageIdentity for package parsing and updated TransformArgs method to conditionally add the version argument. |
src/Cli/dotnet/Commands/Package/Add/PackageAddCommandParser.cs | Added a custom parser for handling package identity with @ syntax and introduced a validator to enforce the disallowance of --version when a version is provided. |
0217d9e
to
3b099aa
Compare
The help message could also reflect the new syntax: - dotnet package add <PACKAGE_NAME> [options]
+ dotnet package add <PACKAGE_NAME>[@<PACKAGE_VERSION>] [OPTIONS] |
this is actually pretty hard to do - S.CL has support for rearranging help sections, but the usage is harder to customize. I did make the help descriptions for the argument call out the new format. |
Agreed. The current way seems to be using the ordered evaluation and to replace the usage, it requires overwriting the delegate in second iteration without actually checking its contents (there is no way to do so?). using System.CommandLine;
using System.CommandLine.Help;
new CliConfiguration(
new MyCommand(args)
.UseExtendedHelp(MyCommand.GetExtendedHelp)
).Invoke(args);
static class Ext
{
// copy of https://github.com/dotnet/runtime/blob/fbb0f14b33394bc55f90de7c2b75c99ce1f9eea6/src/coreclr/tools/Common/CommandLineHelpers.cs#L132-L132
public static CliRootCommand UseExtendedHelp(this CliRootCommand command, Func<HelpContext, IEnumerable<Func<HelpContext, bool>>> customizer)
{
foreach (CliOption option in command.Options)
{
if (option is HelpOption helpOption)
{
HelpBuilder builder = new();
builder.CustomizeLayout(customizer);
helpOption.Action = new HelpAction { Builder = builder };
break;
}
}
return command;
}
}
internal class MyCommand : CliRootCommand
{
public CliArgument<Dictionary<string, string>> InputFilePaths { get; } =
new("input-file-path") { Description = "Input file(s)", Arity = ArgumentArity.OneOrMore };
public CliOption<string> InstructionSet { get; } =
new("--instruction-set") { Description = "SR.InstructionSets" };
public MyCommand(string[] args) : base("SR.CommandShortDescription")
{
Arguments.Add(InputFilePaths);
Options.Add(InstructionSet);
// ...
}
public static IEnumerable<Func<HelpContext, bool>> GetExtendedHelp(HelpContext ctx)
{
int i = 1;
foreach (Func<HelpContext, bool> sectionDelegate in HelpBuilder.Default.GetLayout())
{
if (i++ == 2)
yield return _ =>
{
Console.WriteLine("Usage:\n dotnet package add <PACKAGE_NAME>[@<PACKAGE_VERSION>] [OPTIONS]");
return true;
};
else
yield return sectionDelegate;
}
}
} based on the |
I took a pass at doing just that this morning (marker interfaces on commands to signal that they override the layout, swapping out specific sub-layouts, etc), but there are a lot of layout smarts that are hidden knowledge behind private members in S.CL. It would be quite a lot of code duplication right now :( |
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.
Beautiful PR, thank you!
9e763d6
to
8b87b86
Compare
1393492
to
8fee280
Compare
@dotnet/domestic-cat is something wonky with the fullframework build? It suddenly started throwing tons of test errors unrelated to this PR. |
Looking at another PR, I see:
I'll try to see where that's coming from. |
…sed project lookup as the 'add package' command used to
8fee280
to
d73c742
Compare
Fixes #47981
Adds support for syntax like
[email protected]
indotnet package add
. When this syntax is used, we should disallow the--version
option.This also pushes up a UX fix to the
--project
option of this command - it no longer requires specifying the project in order to work on the current directory. The previous code was trying to make CWD-based usage work but didn't quite get there.