Boost logo

Boost :

From: Vladimir Prus (ghost_at_[hidden])
Date: 2004-12-06 03:03:03


Hi Bertolt,

> Looks like there is not much interest in my patch!?

Quite the opposite, I'm interested in getting this into program_options. I
just seen your first email on Saturday and generally have no time on
weekends for any hacking :-(

Basically, the change is great but I'd like to clarify some things.

1. You wrote:
> - The indent is set as the position of the last tab ('\t') in a paragraph,
> other tabs are ignored.

Could you clarify this? I could not understand this from the example you've
given.

2. I'd prefer that your code be a separate function, which takes a string
and outputs it (or returns a new string with proper line-breaks/indents).

3. Such a function really should have comments. It's very hard to understand
the exact formatting rules from the code, and such documentation must be
present in some form. If you'll provide code comments, I'll eventually
update the documentation. You can just copy all the explanations you've
given in your email to the comment and review it for clarity.

4.

 // we need to use one char less per line to work correctly if actual
 // console has longer lines

Could you clarify?

5.

   // trimm slice if out of bounds
   if (i + slice > e)
   {
      slice -= 1;
   }

Why do you do this?

6.
   // prevent chopped words
   if ((*(i + (slice - 1)) != ' ') &&
      (((i + slice) < e) && (*(i + slice) != ' ')))

This really needs a more verbose comment. Like 'check if current line ends
in non-space, and the next character is non-space too'.

7.
   slice = min<string::difference_type>(line_length - indent, e - i);

Maybe, this should be moved to the top of the loop. I was confused by the
assignment which is not used below.

8. This is completely up to you, but I wonder if consistently using string
indices everywhere would be clearer. Now you use both indices and iterators
and need to convert between them.

Thanks,
Volodya


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk