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

A better Help formatter (V2) #866

Merged
merged 27 commits into from
Oct 7, 2024

Conversation

LostInCompilation
Copy link
Contributor

@LostInCompilation LostInCompilation commented Mar 19, 2023

This is the new PR I've mentioned to work on in PR #858

A better Help Formatter

See below for images of the new help page

Finally, after a lot of planning, understanding CLI11's codebase, testing and coding, the new default Help Formatter is done. There are a lot of changes to make the help page more readable and closer to UNIX standards, see Changelog below for details. One of the highlights is automatic paragraph formatting with correct line wrapping for App and options/flag descriptions as well as the footer.
A goal was to provide more flexibility and better readability for the help page while providing full compatibility with Apps using CLI11 (no breaking changes and no changes to Apps required). Also better support for different terminal sizes. Users can now specify three new optional attributes: right_column_width_, description_paragraph_width_ and footer_paragraph_width_. See code documentation for more details. The different columns for options/flags now scale with the set column_width_ value: Single dash flags occupy 33% of the set column_width_, double dash flags and options (like REQUIRED) 66%. These new attributes allow for indirectly respecting terminal geometry, footer paragraph formatting has also been added (#355). This PR also implements the issues #353 and #856.
The new help page formatting can also be used as an input for man page generation, since it's oriented on the man page style (#413). help2man can be used to generate man pages from help output (see comment down below for example).

I thoroughly tested this code with all possible combinations of flags, options, positionals, subcommands, validators, ...
So far everything works great.
I hope this PR looks good and meets all requirements. I'm looking forward to the implementation of this PR into CLI11. If you have any questions or suggestions feel free to comment.

Fixed/implemented issues by this PR

What about the failing tests?

Of course the tests expect the old help text format. This is why 6 of the tests are failing. Since it is a bit of work to migrate the tests to the new help format, I first wanted to push out this PR and get confirmation before I'll update all the tests.
So please let me know if this PR gets implemented, what changes should be made and then I'll migrate the tests to the new help format, either in this PR or I'll make a new one.

Changelog:

There are no breaking changes. Every App using CLI11 will work with this new formatter with no changes required.

  • Added empty lines at beginning and end of help text
  • Removed double new-line between option groups for consistency. Now all sections have the same number of new-lines
  • Switched usage and description order
  • Only show "Usage"-string if no App name is present. This provides better readability
  • Made categories (Options, Positionals, ...) capital
  • Changed ConfigBase::to_config to correctly process capital "OPTIONS"-group (only affects descriptions of the config file, not a breaking change)
  • Added a paragraph formatter function streamOutAsParagraph to StringTools.hpp
  • Made "description" a paragraph block with correct, word respecting line wrapping and indentation (using the new paragraph formatter function)
  • Made the footer a paragraph block with correct, word respecting line wrapping and indentation
  • Updated documentation for column_width_ to make it more clear
  • Added new member: right_column_width_, added getter and setter for right_column_width_
  • Added new member: description_paragraph_width_, added getter and setter for description_paragraph_width_
  • Added new member: footer_paragraph_width_, added getter and setter for footer_paragraph_width_
  • Positionals description are now formatted as paragraph with correct, word respecting line wrapping
  • Options description are now formatted as paragraph with correct, word respecting line wrapping
  • Short and long options/flags/names are now correctly formatted to always be at the right position (also for subcommand options/flags)
  • Short and long options/flags/names column widths scale linearly with the column_width_ attribute to better adapt to different column_width_ sizes
  • Merged PR Add REQUIRED to an app-name in the --help and --help-all output #860

What's planned for the future?

  • I'm thinking of better formatting the options of flags (like REQUIRED, TEXT, INT, ...) and make them also in a seperate column. This way they would also always be at the same position. However I decided against it for this PR, since I wanted them to be as close as possible to the actual flag. With my implementation it is quite easy to add this change in the future.
  • Subcommands: I'm planning on better formatting the Subcommands. With this PR only the short and long flags/options of subcommands are better formatted (like it is with the main flags, see images down below).
  • Maybe implement a different way to display expected data type options (TEXT, INT, ...). For example: --file-name=<TEXT> for long flags only and if disable_flag_override_ is false.
  • Maybe add something like this: Enum help string formatting #554

Old

CLI_OLD

New

The new help page requires no changes to existing apps using CLI11.
CLI_NEW

@LostInCompilation
Copy link
Contributor Author

LostInCompilation commented Mar 19, 2023

Here is the code used in the example images:

    CLI::App app("A long description\nNew line. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.", "cli-demo-app"); //TUIFM_APP_NAME
    
    app.footer("This is a footer!");

    //********************************************************+
    // Positional
    app.add_option("pos", "A positional");
    app.add_option("longpos", "Consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum.");
    
    // Vars
    std::string s1 = "";
    int i1 = 0;
    
    //********************************************************+
    // Config
    app.set_help_all_flag("--help-all", "Expand all help");
    
    app.add_option("-f,--filename", s1, "A filename");
    app.add_option("-a", "A short name");
    app.add_option("-b", "Another short name");
    app.add_option("-c,-d", i1, "Double short name, integer");
    app.add_option("-r", "Short name with long description. Lorem ipsum dolor sit amet,\nconsetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est.");
    app.add_option("-s", "Short");

    // Info group
    app.set_version_flag("-v,--version", GetAppVersion)->group("INFO");
    
    // MYGROUP1
    app.add_option("-l,--this-is-a-very-long-option-for-testing", "Very long description of a very long flag.")->group("MYGROUP1");
    app.add_option("--only-long", "Only one long name")->group("MYGROUP1");
    app.add_option("--two-longs,--another-long", "Two long names")->group("MYGROUP1");

@LostInCompilation
Copy link
Contributor Author

LostInCompilation commented Mar 19, 2023

help2man and Man pages for CLI11 Apps

This is a man page generated with help2man from the new CLI11 help output (only showing a part of the man page). help2man can be easily used with CLI11 Apps to generate man pages.

To test man page generation for yourself with the new help formatter of CLI11:

  • help2man -o manFile.3 ./cli-demo-app
  • man ./manFile.3

Important: The App must accept the version flag or else help2man will fail: app.set_version_flag("-v,--version", MyGetAppVersion);

226186586-b61cef76-173c-4366-b30a-3d0d4770e27e

@phlptp
Copy link
Collaborator

phlptp commented Mar 20, 2023

First glance I like this a lot! The help2man will be very useful and we probably want a test to ensure things work with that application.
Is there a standard that follows we can reference?

I will take a closer look at this in the coming days and @henryiii should as well.
This looks like it could resolve several outstanding issues.

@phlptp
Copy link
Collaborator

phlptp commented Apr 25, 2023

I think I want @henryiii to comment on this. If he is ok with it, I can help fix the test cases and go through it in more detail.

@henryiii
Copy link
Collaborator

This looks great, happy to see someone making the default formatter closer to existing standards. @phlptp Feel free to move forward!

(Sorry for the long delay, I've been busy with scikit-build-core and other things)

@phlptp
Copy link
Collaborator

phlptp commented Jun 15, 2023

I will start working on fixing some of the tests to adjust for this soon. @LostInCompilation are you still available to help finish this up?

@LostInCompilation
Copy link
Contributor Author

Yes I'm available. Let me know what to do.

@xiota
Copy link

xiota commented Apr 7, 2024

Is there a way to make these improvements available in a separate / optional header file so it doesn't need to be rebased when it isn't merged soon enough? I'm new to this, and help output is oddly important.

@LostInCompilation
Copy link
Contributor Author

Any news on merging this PR? I'm here to help.

@EvanEzell
Copy link

Any updates on merging this in?

@phlptp
Copy link
Collaborator

phlptp commented Sep 27, 2024

I am starting to take a look at this @LostInCompilation. I pushed in the latest changes from main, so getting it up to date, then will start going through the tests and see what needs to be updated.

@phlptp
Copy link
Collaborator

phlptp commented Sep 27, 2024

I think I can get the existing tests to pass with this, but we will need some additional tests with the new methods, and at least a bit of documentation on how to use it well.

Copy link

codecov bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (e4ee3af) to head (7e66ae2).
Report is 39 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##              main      #866     +/-   ##
===========================================
  Coverage   100.00%   100.00%             
===========================================
  Files           17        17             
  Lines         4546      4680    +134     
  Branches         0      1005   +1005     
===========================================
+ Hits          4546      4680    +134     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@phlptp
Copy link
Collaborator

phlptp commented Sep 27, 2024

I think I got everything but the coverage. We will need a few more tests to verify everything is working, and a bit of additional documentation. @LostInCompilation would you be available to add a few more tests and documentation?

@phlptp
Copy link
Collaborator

phlptp commented Oct 4, 2024

There is an issue with the description paragraphs for subcommands. The wrapping and paragraph widths are not honored in that situation. I think that needs to be fixed. Came across that while trying to track down coverage. @LostInCompilation any chance you will able to try to resolve that one.

@phlptp
Copy link
Collaborator

phlptp commented Oct 5, 2024

I think I got this ready to merge, docs and probably some fine tuning on some edge cases can come later. @henryiii I will merge on Monday, unless you have other input.

@phlptp phlptp merged commit 65442ad into CLIUtils:main Oct 7, 2024
55 checks passed
@LostInCompilation
Copy link
Contributor Author

Sorry, I've been away and didn't check GitHub. Thank you for your work and fixing some things! Great to see this merged now. I have to check the documentation from CLI11 again, maybe I can contribute a bit to the docs later.

You have any edge cases in mind I should check out for tweaking fixing?

@LostInCompilation LostInCompilation deleted the BetterHelpFormatter branch October 8, 2024 15:52
@phlptp
Copy link
Collaborator

phlptp commented Oct 8, 2024

A very long list of arguments can create an infinitely long line, which I am not sure is what is desired.
Not entirely sure if aliases for subcommands are handled well yet.
Also not clear whether things like validation strings and some other modifiers are handled well. I haven't checked all that yet. I think the unit tests check that they are there, but not sure the formatting is ideal.

@vadz
Copy link

vadz commented Nov 22, 2024

Sorry, I'm not sure if this is the right place to discuss it, but I've just tried the new help formatter and I have to say that while some changes are nice, others don't seem so at all. In particular, starting the output with $fullpath [OPTIONS] followed by the indented description looks very weird and non-standard to me, all standard Unix utilities follow what CLI11 did before, i.e. put the (non-indented!) description first and then Usage: $fullpath [OPTIONS] after it.

Is there any way to produce the old output by chance?

@LostInCompilation
Copy link
Contributor Author

LostInCompilation commented Nov 22, 2024

Oh yeah I mixed up the order. I can make a PR to fix this.

@LostInCompilation
Copy link
Contributor Author

@vadz See #1093.

phlptp pushed a commit that referenced this pull request Nov 25, 2024
When I rewrote the Help formatter (see #866) I mixed up the order of the
Usage and Description strings. I flipped the order to make it UNIX style
again.
All tests pass.
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.

6 participants