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

Footer callback #309

Merged
merged 1 commit into from
Aug 18, 2019
Merged

Footer callback #309

merged 1 commit into from
Aug 18, 2019

Conversation

phlptp
Copy link
Collaborator

@phlptp phlptp commented Aug 14, 2019

If merged this PR will add a callback for footer generation, to allow extra information into the help screen that can only be generated at run time.

@phlptp
Copy link
Collaborator Author

phlptp commented Aug 14, 2019

This is based on the tuple PR so once that is merged this will go out of draft, or it can be extracted if that direction is not desired.

@codecov
Copy link

codecov bot commented Aug 14, 2019

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #309   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     12           
  Lines        2958   3014   +56     
=====================================
+ Hits         2958   3014   +56
Impacted Files Coverage Δ
include/CLI/FormatterFwd.hpp 100% <ø> (ø) ⬆️
include/CLI/ConfigFwd.hpp 100% <ø> (ø) ⬆️
include/CLI/Formatter.hpp 100% <100%> (ø) ⬆️
include/CLI/Validators.hpp 100% <100%> (ø) ⬆️
include/CLI/Option.hpp 100% <100%> (ø) ⬆️
include/CLI/App.hpp 100% <100%> (ø) ⬆️
include/CLI/TypeTools.hpp 100% <100%> (ø) ⬆️
include/CLI/Split.hpp 100% <100%> (ø) ⬆️
include/CLI/Error.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 4bfce43...472c28d. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 14, 2019

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #309   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     12           
  Lines        3002   3017   +15     
=====================================
+ Hits         3002   3017   +15
Impacted Files Coverage Δ
include/CLI/FormatterFwd.hpp 100% <ø> (ø) ⬆️
include/CLI/ConfigFwd.hpp 100% <ø> (ø) ⬆️
include/CLI/Formatter.hpp 100% <100%> (ø) ⬆️
include/CLI/Validators.hpp 100% <100%> (ø) ⬆️
include/CLI/Option.hpp 100% <100%> (ø) ⬆️
include/CLI/App.hpp 100% <100%> (ø) ⬆️
include/CLI/Split.hpp 100% <100%> (ø) ⬆️
include/CLI/Error.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 127f538...f507f3e. Read the comment docs.

fix incorrect parenthesis

update some clang-tidy fixes mainly else after return but a few conversions from into to bool

add extra newline before footer

add an extra field to the extra Error

add a footer callback for help operations
@phlptp phlptp marked this pull request as ready for review August 16, 2019 15:36
@henryiii
Copy link
Collaborator

Should there be a footer and a footer callback? Can't the footer sting function just make a callback that returns the string given? Other than that it looks good.

@phlptp
Copy link
Collaborator Author

phlptp commented Aug 18, 2019

My initial thought was to do that as well, but the issue with that is footer as a string is inheritable, and I don't think you want a callback to be inheritable, So there really isn't a good resolution between them other than leaving both in place.

see the gitter question a few months ago. I thought about making the callback inheritable but that has issues as well. The use case I needed it for was a cascading app, so I do some processing, then possibly depending on options called another processing app (embedded in another class) is called to process some more command line arguments. I wanted the help to actually generate help for all of them. So the solution was the footer callback (which we have been using for a couple months now), which would call the second app inside the footer callback to generate the help for the additional options inside the footer. Eventually I hope there is a cleaner way that could be done but it works pretty well for the moment when some of the help is dynamic. And I don't think that would work or even make sense if that callback were passed to the subcommands.

@henryiii henryiii merged commit 06ab2d0 into CLIUtils:master Aug 18, 2019
@phlptp phlptp mentioned this pull request Aug 19, 2019
@phlptp phlptp deleted the footer_callback branch September 4, 2019 03:23
@henryiii henryiii added this to the v1.9 milestone Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants