Skip to content

Missing overloads in the System.CommandLine.Handler.SetHandler extension methods #10846

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

Closed
nobledog opened this issue Jan 16, 2025 · 3 comments · Fixed by #11172
Closed

Missing overloads in the System.CommandLine.Handler.SetHandler extension methods #10846

nobledog opened this issue Jan 16, 2025 · 3 comments · Fixed by #11172
Assignees
Labels
area-System.Runtime 📌 seQUESTered Identifies that an issue has been imported into Quest.

Comments

@nobledog
Copy link

nobledog commented Jan 16, 2025

Type of issue

Other (describe below)

Description

In the open source Handler.Func.cs file the definitions for methods of the from public static void SetHandler<T1, T2, ..., Tn>(...) only run up to T8 while the documentation shows them running up to T16. Too bad for me, I wrote a call that required 10 such parameters.

I am not sure whether it would be better to leave my feedback to this or add the missing methods and put it in a pull request. Happy to do that. Please let me know. For now I will code up a workaround.

[Enter feedback here]

Page URL

https://learn.microsoft.com/en-us/dotnet/api/system.commandline.handler.sethandler?view=system-commandline#system-commandline-handler-sethandler(system-commandline-command-system-action)

Content source URL

https://github.com/dotnet/dotnet-api-docs/blob/main/xml/System.CommandLine/Handler.xml

Document Version Independent Id

5737109f-d286-b752-0c0d-122a2cf74e34

Article author

@dotnet-bot


Associated WorkItem - 418879

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 16, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime

@gewarren
Copy link
Contributor

gewarren commented Feb 3, 2025

@nobledog This sounds like a product issue, not a docs issue. Is that correct?

@nobledog
Copy link
Author

nobledog commented Feb 6, 2025

Hmm, let me give you some background and can decide whether its a doc or product issue.

Apologies if this is longer winded than you would want or can even conceive of. I am not sure how much input you expect and/or whether you might be one of CommandLine's coders or engineers who might want more details.

Subsequent to sending in the comment you are responding to, I found another place in the online docs that explained an alternative approach to implementing handlers that require many arguments. It involves using a SetHandler method that has the signature shown below (taken from Handler.Func.cs around line 26):

/// <summary>
/// Sets a command's handler based on a <see cref="Func{Task,InvocationContext}"/>.
/// </summary>
public static void SetHandler( this Command command,   Func<InvocationContext, Task> handle) =>
    command.Handler = new AnonymousCommandHandler(handle);

The nearest method to this in the current documentation looks like:

 SetHandler<T>(Command, Func<T,Task>, IValueDescriptor[])

Indeed it defined directly below the ither one in the source file.

Presumably, in code elsewhere in the CommandLine package that I haven't bothered to track down the Command class knows how to pass an InvocationContext to this type of delegate instead of a whole bunch of generically parameterized arguments. The InvocationContext allows one to reach any Options or Arguments defined on the Command object using a linkage from the InvocationContext to its ParseResult and to that object's GetValueForOption(Option) method.

While it is nice to have some of the other overloads for passing Options/Arguments directly to the handler, the doc presently lists variants supporting as many as 16 of them. However, only around 8 are actually implemented in Handler.Func.cs. This is what led to my initial report. I was still trying to figure out a good workaround. I realize CommandLine is still pre-release so I am not complaining. But I am sure you will want the doc issue cleaned up prior to release.

I am guessing that maybe the implementers decided that having such a bevy of overloads was silly past 8 and added the alternate
approach with the InvocationContext to avoid the proliferation. Fact is, it's a much better solution as far as I am concerned.

Other than there being those missing overloads all the code seems to work perfectly find and, to reiterate, I think, the added method was sound design.

I stumbled upon the InvocationContext approach on the learn.microsoft.com site. The relevant page is:

   https://learn.microsoft.com/en-us/dotnet/standard/commandline/model-binding

The section is called "How to bind arguments to handlers in System.CommandLine". It describes the InvocationContext approach and it also correctly remarks on the more limited set of overloads that actually exist. So, I think it's mostly a question of updating the doc at 'https://learn.microsoft.com/en-us/dotnet/api/system.commandline.handler.sethandler'. Seems more a doc issue after all.

For what it is worth, please tell the implementers I think they did excellent work. I am not using it in any extremely intensive way but it has worked well and, except for my one little problem long since solved, has proven easy to use and more than sufficiently capable. I was not needing some kind of getop, after all though I suppose it could grow into something hoary and venerable like that.

Let me know if you need any more information. Regards, CM

@gewarren gewarren self-assigned this Apr 7, 2025
@gewarren gewarren added the 🗺️ reQUEST Triggers an issue to be imported into Quest. label Apr 7, 2025
@gewarren gewarren moved this from 🔖 Ready to 🏗 In progress in dotnet/docs April 2025 sprint project Apr 7, 2025
@gewarren gewarren moved this from 🏗 In progress to 👀 In review in dotnet/docs April 2025 sprint project Apr 8, 2025
@sequestor sequestor bot added 📌 seQUESTered Identifies that an issue has been imported into Quest. and removed 🗺️ reQUEST Triggers an issue to be imported into Quest. labels Apr 8, 2025
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in dotnet/docs April 2025 sprint project Apr 8, 2025
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Runtime 📌 seQUESTered Identifies that an issue has been imported into Quest.
Projects
Development

Successfully merging a pull request may close this issue.

2 participants