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

Add unnamed subcommand #216

Merged
merged 1 commit into from
Feb 6, 2019
Merged

Conversation

phlptp
Copy link
Collaborator

@phlptp phlptp commented Feb 4, 2019

If merged this pull request will allow unnamed subcommands that get checked for options if no matches occur in the main App. it also allows the extraction and insertion of subcommands via shared_ptr.

See issue #215.

@phlptp
Copy link
Collaborator Author

phlptp commented Feb 4, 2019

A couple remaining issues
Should there be a way to remove and or disable subcommands?
Need to update the Readme yet.

@codecov
Copy link

codecov bot commented Feb 4, 2019

Codecov Report

Merging #216 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #216   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     12           
  Lines        1960   2023   +63     
=====================================
+ Hits         1960   2023   +63
Impacted Files Coverage Δ
include/CLI/App.hpp 100% <100%> (ø) ⬆️
include/CLI/Formatter.hpp 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4910df...255d1a2. Read the comment docs.

include/CLI/App.hpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@henryiii henryiii left a comment

Choose a reason for hiding this comment

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

Another idea would be to have two lists of subcommands, named and unnamed, and then looping over one or the other as needed.

include/CLI/App.hpp Outdated Show resolved Hide resolved
include/CLI/App.hpp Outdated Show resolved Hide resolved
include/CLI/App.hpp Show resolved Hide resolved
@phlptp
Copy link
Collaborator Author

phlptp commented Feb 5, 2019

Todo yet

  • combine all the commits to a single commit
  • update the readme

@phlptp
Copy link
Collaborator Author

phlptp commented Feb 5, 2019

Question. Should the parsed_ flag on the nameless subcommands be set. Right now they are not but it wouldn't be hard to set the nameless subcommands when the parsed_ flag is set in the main App?

@phlptp
Copy link
Collaborator Author

phlptp commented Feb 5, 2019

Right now the nameless subcommands get printed with help before a list of subcommands. If a subcommand has a non-default group definition should it do something with that or should the group information on a nameless subcommand be ignored?

@phlptp
Copy link
Collaborator Author

phlptp commented Feb 5, 2019

@henryiii If the latest commit passes Codecov I will merge everything to a single commit, unless you see some other things that should be changed.

include/CLI/App.hpp Outdated Show resolved Hide resolved
include/CLI/App.hpp Show resolved Hide resolved
include/CLI/App.hpp Show resolved Hide resolved
@henryiii
Copy link
Collaborator

henryiii commented Feb 5, 2019

Add a few small last minute suggestions you can look at. I'm not sure what an unnamed subcommand should do for "parsed" - it rather seems like setting it to 1 when it is checked (that is, all unnamed subcommands will get a 1) might be an option. Really, a user shouldn't be checking the "count" of an unnamed subcommand, so the only effect would be in the parsing, where it seems like it wasn't needed for tracking unnamed subcommands.

@henryiii
Copy link
Collaborator

henryiii commented Feb 5, 2019

I think we can tweak the formatting if needed on nameless subcommands after the merge, do what you think is best for now.

update the readme, and add a formatter test for nameless subcommands in nondefault group with other named subcommands.

add a test of default arguments

add a formatter test

add tests for unnamed subcommands and an example of the partitioned subcommands.

change the app_p to be a shared_ptr so you can add an App later on and merge them together

add the ability to add unnamed subcommands that allow partitioning on options into multiple apps.
@henryiii henryiii merged commit 598046c into CLIUtils:master Feb 6, 2019
@henryiii
Copy link
Collaborator

henryiii commented Feb 6, 2019

Thanks!

@phlptp phlptp deleted the add_unnamed_subcommand branch February 6, 2019 15:10
@henryiii henryiii added this to the v1.8 milestone Feb 9, 2019
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