Re: [PATCH 3/2] pretty: support right alignment

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 21, 2012, 17:46 UTC
Message-ID: <7vipb75kvk.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CACsJy8AAjxMN7MX09Eq4Dy6NJHMkyxGJZm9uZquXWTi0goAYLQ@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

Show 17 quoted lines

>>  - How does this interact with the wrapped output?  Should it?
>
> We have to deal with it anyway when the left aligned text takes all
> the space. On one hand, I don't want to break the terminal width,
> leading to ugly output, so it'll interact. On the other hand, I don't
> really wish to turn pretty format machinery into a full feature text
> layout engine (by ripping of links/lynx?). So we have a few options:
>
> 1. ellipses, line cutting means i18n issues ahead
> 2. just put the right-aligned text on another line. We do something
> similar in parse-options. When the option syntax is too long, we put
> help description on the next line.
> 3. bring in html/css box model for arranging text so that both
> left/right aligned texts can share the same line.
> 4. tell users upfront it's not supported. don't do that
>
> I'd vote 2, or 4.

I am fine with 4 personally.

Show 20 quoted lines

>>  - I am wondering if somebody ever want to do this with a follow-up
>>    patch:
>>
>>         Left %h%|Center %cd%|Right %ad
>>
>>    Is %| a sensible choice for "flush right"?  I am wondering if it
>>    makes more sense to make %|, %< and %> as "multi-column
>>    introducer" (the example defines output with three columns) that
>>    also tells how text inside each column is flushed inside the
>>    column, e.g.
>>
>>         %>col 1 right flushed%|col 2 centered%< col 3 left flushed
>>
>>    or something like that (we may want explicit "column width"
>>    specifiers if we were to do this kind of thing).
>
> Yeah that crossed my mind. But I'll need to convince myself it's
> actually useful. Once you're on that road, you may want >=4 column
> tables. We can extend column.c to do that. That hard part is
> converting pretty format to use column functions.

Reading the above again, I realize that I may have sounded as if saying "With 'flush to right', we are inviting wishes for 'left' and 'center', and a patch that only does 'right' is unacceptable.", but that was not what I meant. I am perfectly fine with 'flush to right' without anything else as the first step.

The only concern I had was that somebody who later tries to add 'left', 'center', etc. to accompany the 'right' that you are adding with your patch will find it unfortunate that the natural choice for 'center', i.e. %|, has been taken to mean a wrong thing and that mistake cannot be corrected without breaking backward compatibility.