dotnet / command-line-api

Command line parsing, invocation, and rendering of terminal output.
https://github.com/dotnet/command-line-api/wiki
MIT License
3.35k stars 375 forks source link

[Proposal] Remove `TreatUnmatchedTokensAsErrors` #2350

Open KathleenDollard opened 4 months ago

KathleenDollard commented 4 months ago

In the same spirit as asking "why" on custom parsers, we are asking "why" on TreatUnmatchedTokensAsErrors.

The major reason, and the one used by dotnet new and dotnet msbuild is to compose CLIs. The args not used by the .NET CLI are passed to the additional parser. In the case of dotnet new it builds a dynamic parser based on the template chosen (very cool stuff). msbuild still has its own parser.

We've found that this is better handled by creating an CliArgument that is an array of strings and passing that array along to the other parser.

One of the reasons TreatUnmatchedTokensAsErrors feels uncomfortable because it leads to exposing the Token type outside the parser layer. It seems preferable to make this an internal type. There also may be an implication that you can have unmatched tokens anywhere in the CLI, but in general, once unmatched tokens appear, parsing can become complicated and problematic.

The proposal is to remove it.

We look forward to hearing about any scenarios that are not covered by a collection of strings as an argument on the command that should route to a composed CLI.

KalleOlaviNiemitalo commented 4 months ago

The dotnet msbuild strategy of creating an CliArgument that is an array of strings, fails to distinguish between these commands:

In dotnet new, I don't think templates can take non-option arguments, so the ambiguity is less of a problem there.

It could still become a problem in other applications that dynamically build parsers. But that doesn't mean TreatUnmatchedTokensAsErrors is the best way to fix it. Some kind of parser hook that lets the application create a subcommand on demand could also work.

KathleenDollard commented 3 months ago

@baronfel @rainersigwald

Can you comment on this - how we do it, how we want to do it, and whether we could make a change without breaking the world.

KathleenDollard commented 3 months ago

Does anyone besides dotnet .. have the need to modify the CLI tree based on commands within the tree?

Background:

jonsequitur commented 3 months ago

The usual use case for this that I've seen is that after a certain point on the command line, a second parser will be taking over, possibly in another process.

dotnet (incorrectly 🤓) used -- as the signal for this hand-off but not all command lines make it explicit. More often than not, it's associated with a subcommand (e.g. dotnet new which can hand off to an effectively infinite number of different parsers for the different templates.)

baronfel commented 3 months ago

FWIW we've long since fixed our incorrect usage of -- and moved to standard handling of -- for the most relevant commands of new and run. I think the driver for this was a pre-beta-4 S.CL processing change that kind of forced us to get correct! (Pay no attention to the fact that several tools contributed by other teams (watch, razor, dotnet-test) are still doing things incorrectly).

tmat commented 3 months ago

@KathleenDollard We need this API in dotnet watch: See https://github.com/dotnet/sdk/pull/39618.

There we are parsing a command line with unknown structure and passing some arguments through to an arbitrary subcommand that dotnet-watch invokes.

I'd prefer better APIs to do so, one that would be easier to use and handle -- properly (same issue as https://github.com/dotnet/command-line-api/issues/2350#issuecomment-1986959979).

Perhaps having a special CliCommand that servers as a placeholder for an arbitrary command and all options and arguments following it get associated with it might work.

KathleenDollard commented 3 months ago

The alternative approach to unmatched tokens is to create a command with an argument that is a collection of strings. Those strings would then be the args passed to the command.

I believe that -- could be treated as a command for this purpose if that is sensible.

I am looking for cases where that approach would fail. We will definitely support handing off to other parsers.

Perhaps having a special CliCommand that servers as a placeholder for an arbitrary command and all options and arguments following it get associated with it might work.

Just want to clarify whether this is just making it easy to create CliCommand with an argument that is a collection of strings, or something fancier.