Boost C++ Libraries: Ticket #3703: Request for ability to specify description column width

attachment set

Chard — Tue, 01 Dec 2009 10:26:03 GMT

attachment → Ticket_#3703.zip

example specifying description column width; #3703

cc set

Sascha Ochsenknecht — Tue, 01 Dec 2009 14:06:10 GMT

cc s.ochsenknecht@… added

Sascha Ochsenknecht — Tue, 01 Dec 2009 14:51:52 GMT

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

attachment set

Chard — Tue, 01 Dec 2009 23:52:45 GMT

attachment → Ticket_#3703.2.zip

Update for #3703

severity changed

Chard — Tue, 01 Dec 2009 23:53:29 GMT

severity Problem → 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 ;-).

owner, status changed

Sascha Ochsenknecht — Wed, 02 Dec 2009 12:12:34 GMT

owner changed from Vladimir Prus to Sascha Ochsenknecht
status new → 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.

Sascha Ochsenknecht — Wed, 02 Dec 2009 12:16:45 GMT

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.

status changed; resolution set

Sascha Ochsenknecht — Wed, 02 Dec 2009 13:35:56 GMT

status assigned → closed
resolution → fixed

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

Sascha Ochsenknecht — Wed, 02 Dec 2009 13:37:02 GMT

I just added the patch with some small modifications.

Please review!

Thanks, SAscha

Wed, 02 Dec 2009 15:36:15 GMT

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...

Thu, 03 Dec 2009 10:05:49 GMT

Hi Sascha

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

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");
    }

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.

Sascha Ochsenknecht — Thu, 03 Dec 2009 10:42:37 GMT

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:

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.

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