Skip to content

Add default verb support#556

Closed
Artentus wants to merge 0 commit into
commandlineparser:developfrom
Artentus:develop
Closed

Add default verb support#556
Artentus wants to merge 0 commit into
commandlineparser:developfrom
Artentus:develop

Conversation

@Artentus

Copy link
Copy Markdown
Contributor

I added basic support for a default verb (a verb that will be assumed if no verb is specified).
Tests all completed on my end but I'm not too knowledgable about this code, if there is any problem let me know.

@moh-hassan

Copy link
Copy Markdown
Collaborator

Thanks @Artentus for your work.
I did a quick test:

  • for a default verb, it works as expected.
  • With more than default verb (not allowed), an Exception "Sequence contains more than one matching element" is fired.
    You can validate that only one verb has a default and fire error,e.g, "More than one default verb is not allowed"
  • Modify the HelpText screen "--help" to show the help with indication of the default verb, e.g

read (Default verb) Reads from source.
write Writes to destination.

  • Add test cases to cover the above cases.

@Artentus

Artentus commented Jan 4, 2020

Copy link
Copy Markdown
Contributor Author

The error with more than one default verb is expected. However I do agree a more descriptive error message could be useful.

As for the help text, I'll first have to see how that is actually implemented before I can modify it. Will see what I can do.

@moh-hassan

Copy link
Copy Markdown
Collaborator

As for the help text, I'll first have to see how that is actually implemented before I can modify it. Will see what I can do.

You can concentrate in the error handling and its unit test and push the modification, so I can start review the PR.

@Artentus

Artentus commented Jan 5, 2020

Copy link
Copy Markdown
Contributor Author

Done, there is now proper error handling and a couple of new unit tests.

@moh-hassan moh-hassan self-assigned this Jan 5, 2020
Comment thread src/CommandLine/VerbAttribute.cs Outdated
Comment thread src/CommandLine/Core/Verb.cs Outdated
Comment thread src/CommandLine/Core/Verb.cs Outdated
Comment thread src/CommandLine/VerbAttribute.cs Outdated
@moh-hassan

Copy link
Copy Markdown
Collaborator

It's nice if you git rebase to origin\develop to sync the last changes in develop branch.

@Artentus

Copy link
Copy Markdown
Contributor Author

Sorry it took me so long, have been busy this week, but everything is merged now.

I see you don't want to allow default verbs with no name. My reasoning behind this was to just have it behave like no verb at all. Functionally it doesn't change anything tho.

@moh-hassan

moh-hassan commented Jan 14, 2020

Copy link
Copy Markdown
Collaborator

Thanks @Artentus for Update.
PR is approved and ready to merge

@moh-hassan

Copy link
Copy Markdown
Collaborator

Merged manually to resolve merge confliction at commit 4bed3432c

@moh-hassan moh-hassan closed this Feb 3, 2020
@moh-hassan moh-hassan added this to the 2.8 milestone Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants