Skip to content

formatter: add long_option_alignment_ratio#1185

Merged
phlptp merged 2 commits into
CLIUtils:mainfrom
radimkrcmar:long-option-alignment-ratio
Aug 27, 2025
Merged

formatter: add long_option_alignment_ratio#1185
phlptp merged 2 commits into
CLIUtils:mainfrom
radimkrcmar:long-option-alignment-ratio

Conversation

@radimkrcmar
Copy link
Copy Markdown
Contributor

The default formatter has hardcoded ratio at which the long options are aligned. It's currently 1/3 of the column, which makes the default look awkward:

  -h,     --help              Print
          --option            Something

A ->long_option_alignment_ratio(6/30.f) allows output to look like this:

  -h, --help                  Print
      --option                Something

The 1/3 ratio is also bad if you want to print "descriptive" long options on a single line, because then you might want to increase the column width, but that means you waste more space on short options.

e.g. ->column_width(46)

  -l,          --very-descriptive-long-option  Something

vs. ->column_width(38)

  -l,       --very-descriptive-long-option
                                      Something

vs. ->column_width(38) ->long_option_alignment_ratio(6/38.f)

  -l, --very-descriptive-long-option  Something

Any absolute offset X can be set as X/column_width, so provide a ratio-based interface.

I would have prefered to give an absolute integer offset, but we still have to preserve the functionality that does 1/3 if user changed nothing, which means that ratio-based interface is simpler.

I don't have a good idea for the name, "short_option_ratio" might work as well.
The setter does not sanity check that the value is in [0;1] range.

Copilot AI review requested due to automatic review settings July 23, 2025 08:45
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds configurable alignment for long options in the CLI formatter by introducing a long_option_alignment_ratio parameter. The change replaces the hardcoded 1/3 ratio with a customizable float value, allowing better control over the visual alignment of short and long options in help text.

Key changes:

  • Adds long_option_alignment_ratio_ member variable with default value of 1/3
  • Replaces hardcoded column width calculations with ratio-based calculations
  • Provides setter method for configuring the alignment ratio

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
include/CLI/FormatterFwd.hpp Adds new member variable and setter method for long option alignment ratio
include/CLI/impl/Formatter_inl.hpp Updates column width calculations to use the configurable ratio instead of hardcoded 1/3
tests/FormatterTest.cpp Adds test case to verify the new alignment functionality works correctly

Comment thread include/CLI/FormatterFwd.hpp Outdated
std::size_t column_width_{30};

/// The alignment ratio for long options within the left column
float long_option_alignment_ratio_{1/3.0f};
Copy link

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

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

The expression 1/3.0f uses integer division before conversion to float. Use 1.0f/3.0f to ensure proper floating-point division.

Suggested change
float long_option_alignment_ratio_{1/3.0f};
float long_option_alignment_ratio_{1.0f/3.0f};

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, the expression first promotes to float, but I can use 1.f / 3.f if that fits the coding style better.

Comment thread tests/FormatterTest.cpp Outdated

/// Set the alignment ratio for long options within the left column
/// The ratio is in [0;1] range (e.g. 0.2 = 20% of column width, 6.f/column_width = 6th character)
void long_option_alignment_ratio(float ratio) { long_option_alignment_ratio_ = ratio; }
Copy link

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

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

The setter method lacks input validation to ensure the ratio is within the documented [0;1] range, which could lead to unexpected formatting behavior.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am aware, do we want to complicate CLI11 by checking?

The default formatter has hardcoded ratio at which the long options are
aligned.  It's currently 1/3 of the column, which makes the default look
awkward:

  -h,     --help              Print
          --option            Something

A ->long_option_alignment_ratio(6/30.f) allows output to look like this:

  -h, --help                  Print
      --option                Something

The 1/3 ratio is also bad if you want to print "descriptive" long
options on a single line, because then you might want to increase the
column width, but that means you waste more space on short options.

e.g. ->column_width(46)

  -l,          --very-descriptive-long-option  Something

vs. ->column_width(38)

  -l,       --very-descriptive-long-option
                                      Something

vs. ->column_width(38) ->long_option_alignment_ratio(6/38.f)

  -l, --very-descriptive-long-option  Something

Any absolute offset `X` can be set as `X/column_width`, so provide a
ratio-based interface.

I would have prefered to give an absolute integer offset, but we still
have to preserve the functionality that does 1/3 if user changed
nothing, which means that ratio-based interface is simpler.

I don't have a good idea for the name, "short_option_ratio" might work
as well.
The setter does not sanity check that the value is in [0;1] range.

Signed-off-by: Radim Krčmář <radim@krcmar.dev>
@radimkrcmar radimkrcmar force-pushed the long-option-alignment-ratio branch from ce8a42f to 40efc70 Compare July 23, 2025 14:24
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (e4ee3af) to head (7df9d9c).
⚠️ Report is 115 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##              main     #1185     +/-   ##
===========================================
  Coverage   100.00%   100.00%             
===========================================
  Files           17        17             
  Lines         4546      5171    +625     
  Branches         0      1062   +1062     
===========================================
+ Hits          4546      5171    +625     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@phlptp
Copy link
Copy Markdown
Collaborator

phlptp commented Jul 25, 2025

Would we want to suggest different defaults with this new configuration options?

@radimkrcmar
Copy link
Copy Markdown
Contributor Author

That changes behavior (which will annoy users), so I tried to to be as conservative as possible.

If we accepted backward incompatibility, I'd much rather redefine the option to be an integer offset, so it wouldn't matter what is the width of the left column, and short options would by default be assumed to be single letter.

That is, I'd personally prefer if the default looks like this, regardless of what user set to column_width():

  -f, --foo
      --bar

@phlptp
Copy link
Copy Markdown
Collaborator

phlptp commented Aug 7, 2025

I don't really want to break API compatibility at this point, but maybe when we get to a 3.0 version. Also there is some possibility of short options being longer with the allowance for non-standard option names now.

@phlptp
Copy link
Copy Markdown
Collaborator

phlptp commented Aug 21, 2025

@henryiii Any comments on this one, otherwise I will merge it in a few days.

@phlptp phlptp merged commit e7e8de0 into CLIUtils:main Aug 27, 2025
61 checks passed
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.

3 participants