Modify

Ticket #3703 (closed Feature Requests: fixed)

Opened 4 years ago

Last modified 4 years ago

Request for ability to specify description column width

Reported by: Chard Owned by: s_ochsenknecht
Milestone: Boost 1.42.0 Component: program_options
Version: Boost 1.41.0 Severity: Cosmetic
Keywords: description column formatting Cc: s.ochsenknecht@…

Description

I see that the version of options_description.cpp on the trunk has been modified to address the issue where long options cause the description text to be bunched up on the right.

The trunk version is using the (sensible assumption) of the (line_length / 2) as the threshold.

This is a request to expose this threshold position by providing another constructor (or mutator); the programmer may want a different ratio between the option:description columns.

Patches are included that demonstrate one possible approach (using another constructor).

Notes:

The original constructors were modified to use the (line_length / 2) threshold. It could be argued that the default should be 1; to preserve current (1.41) display behaviour.

The original code has a constant of 23 in the code, for the minimum description column start position. In the patch this wins even if the min_description_length would encroach into this area. I wasn't sure whether this should be asserted in the constructor, i.e. assert(min_description_length < m_lineLength - (23U + 1U)).

There is also an options_description_test.cpp patch to demonstrate the behaviour.

(Also: are you aware that test_long_default_value() asserts?)

Attachments

Ticket_#3703.zip Download (2.6 KB) - added by Chard 4 years ago.
example specifying description column width; #3703
Ticket_#3703.2.zip Download (2.3 KB) - added by Chard 4 years ago.
Update for #3703

Change History

Changed 4 years ago by Chard

example specifying description column width; #3703

comment:1 Changed 4 years ago by s_ochsenknecht

  • Cc s.ochsenknecht@… added

comment:2 in reply to: ↑ description ; follow-up: ↓ 3 Changed 4 years ago by s_ochsenknecht

Hi Chad,

thanks for submitting the ticket+patch. I'll have a look to it.

Replying to Chard:

(Also: are you aware that test_long_default_value() asserts?)

Hm, not on my platform (g++ 4.0.2, x86_64) with current trunk version. What is the output?

Thanks, Sascha

Changed 4 years ago by Chard

Update for #3703

comment:3 in reply to: ↑ 2 Changed 4 years ago by Chard

  • Severity changed from Problem to Cosmetic

Replying to s_ochsenknecht:

thanks for submitting the ticket+patch. I'll have a look to it.

Actually, that demo code was a mess. I've attached an updated version that is much cleaner.

The new version shows that the only change required is the introduction of the constructor (or we could have a mutating method), and a one-line change to the trunk code where it calculates the column width.

(Also: are you aware that test_long_default_value() asserts?)

Hm, not on my platform (g++ 4.0.2, x86_64) with current trunk version. What is the output?

My bad, sorry. This doesn't occur with the update, hopefully ;-).

comment:4 Changed 4 years ago by s_ochsenknecht

  • Owner changed from vladimir_prus to s_ochsenknecht
  • Status changed from new to assigned

I like the parameter to configure the space for the description text. I already thought about when fixing #2613, but didn't commit it.

I think I'll add the patch with some slight modifications.

But one question, I saw you added a 'caption' parameter? What is the usage of this parameter? I would remove it, specially for the checkin for this ticket.

comment:5 Changed 4 years ago by s_ochsenknecht

ups sorry, the caption parameter was there before ...

Maybe can you add patch as one single text file? The we can see the diffs directly in trac system. Use svn diff. Thanks.

comment:6 Changed 4 years ago by s_ochsenknecht

  • Status changed from assigned to closed
  • Resolution set to fixed

(In [58095]) Additional parameter to allow user to specify width of column for description text, patch from Chard, Fixes #3703

comment:7 Changed 4 years ago by s_ochsenknecht

I just added the patch with some small modifications.

Please review!

Thanks, SAscha

comment:8 Changed 4 years ago by Chard <boost@…>

Nice one.

Modifying the original constructors is much better; it allows us to use default args so the user gets visibility of the default behaviour. I wasn't sure if I was allowed to touch them!

Thanks...

comment:9 follow-up: ↓ 10 Changed 4 years ago by Chard <boost@…>

Hi Sascha

I've had a bit more of a look at the changes, and there's a couple of points to note:

  1. I had called the original description length variable: min_description_length, this has been renamed to description_length. The "min_" part does add some semantic meaning to the usage because the length that is specified is not an absolute.

For example, if the user specifies 10 as the description length, he may be expecting the description column to occupy exactly 10 spaces. But the available distance is still determined by the length of the option column, as demonstrated by this test case:

    { 
        options_description desc("", 
                                 options_description::m_default_line_length, 
                                 10U); 
        desc.add_options() 
            ("lt-23-chars", new untyped_value(), 
            "this shows that more space is given to the description column when possible") 
        ; 
        stringstream ss; 
        ss << desc; 
        BOOST_CHECK_EQUAL(ss.str(), 
       //123456789_123456789_1234 
        "  --lt-23-chars arg     this shows that more space is given to the description \n" 
        "                        column when possible\n"); 
    }
  1. By tying the default argument for the description length to (default_line_length / 2) it does mean that existing code that specifies the line_length argument will still compile, but with a potentially different display. The minimum description width will still be 40, regardless of the specified line_length. However, I suspect this is a minor point because for most cases the output should look better.

Regards, Chard.

comment:10 in reply to: ↑ 9 Changed 4 years ago by s_ochsenknecht

Replying to Chard <boost@…>:

Hi Sascha

I've had a bit more of a look at the changes, and there's a couple of points to note:

  1. I had called the original description length variable: min_description_length, this has been renamed to description_length. The "min_" part does add some semantic meaning to the usage because the length that is specified is not an absolute.

Yes, you're right. I agree that min_description_length might be the better name here. I wasn't 100% aware that the description could be longer. I'll change that.

  1. By tying the default argument for the description length to (default_line_length / 2) it does mean that existing code that specifies the line_length argument will still compile, but with a potentially different display. The minimum description width will still be 40, regardless of the specified line_length. However, I suspect this is a minor point because for most cases the output should look better.

Yes, I wouldn't change it now since it can not be detected within the constructor if the default parameter was used or not. Or we end up with 1 or more constructor overloads, which I would like to avoid. Most likely the user will specify all parameters or none, I guess.

Thanks & regards, Sascha

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.