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

Use System.CommandLine to parse cmdline args and allow provider to offer cmds #273

Merged
merged 2 commits into from
Feb 5, 2021

Conversation

mjcheetham
Copy link
Collaborator

Replace our basic custom command-line handling code with the System.CommandLine library which provides simpler and easier handling.

Add the ability for host providers to register themselves as offering custom commands, under the provider ID name.

For example a provider with the ID 'foo' would be able to expose commands under the git-credential-manager-core foo <..> command.

Copy link
Contributor

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

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

There is certainly a lot going on here, but I appreciate that the tests get a bit simpler.

I hope this makes things easier as you extend into other features.

src/shared/Microsoft.Git.CredentialManager/Application.cs Outdated Show resolved Hide resolved
Comment on lines 5 to 14
public interface ICommandProvider
{
/// <summary>
/// Configure a custom provider command.
/// </summary>
/// <param name="rootCommand">Root provider command.</param>
void ConfigureCommand(Command rootCommand);
}

internal class ProviderCommand : Command
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little confused as to why these are in the same file. They are not using each other in any visible way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's because I'm an idiot 😜

I changed the extension point design here a few times, but this doesn't "feel" right. I've moved to a factory style now where an ICommandProvider implementing host provider returns a new ProviderCommand instance from the CreateCommand() method.

Replace our basic custom command-line handling code with the
System.CommandLine library which provides simpler and easier handling.
Add the ability for host providers to register themselves as offering
custom commands, under the provider ID name.

For example a provider with the ID 'foo' would be able to expose
commands under the `git-credential-manager-core foo <..>` command.
@mjcheetham
Copy link
Collaborator Author

There is certainly a lot going on here, but I appreciate that the tests get a bit simpler.

Yes, when we only had the get|erase|store commands for Git's consumption just a simple in-house parser was appropriate. Now we're adding more commands with options, arguments etc it feels the correct time to make this someone else's problem. 😉

I hope this makes things easier as you extend into other features.

That's the plan!

@mjcheetham mjcheetham merged commit 197817b into git-ecosystem:master Feb 5, 2021
@mjcheetham mjcheetham deleted the cmdline branch February 5, 2021 11:43
@mjcheetham mjcheetham mentioned this pull request Mar 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants