Re: [PATCH v4 0/5] Better ref summary alignment in "git fetch"

From: Duy Nguyen <pclouds@gmail.com>
Date: Jun 30, 2016, 16:16 UTC
Message-ID: <CACsJy8DS3n=7sRGztqe16igrsVaKt3Qa-GRVJBMoZvtEJmdmQA@mail.gmail.com>
In-Reply-To: <20160627184354.GA9594@sigill.intra.peff.net>

On Mon, Jun 27, 2016 at 8:43 PM, Jeff King <peff@peff.net> wrote:

Show 20 quoted lines

> I tried it on my most-horrible example case, and the results were...just
> OK. Because the variable-length part of each line comes first, the
> alignment code means that the "origin/$" bit of every line gets bumped
> out. And if you have a single large branch name, then everybody gets
> bumped out very far, even to the point of wrapping. E.g., I get
> something like (with fetch.output=compact, obviously):
>
>   From ...
>    * [new branch]      branch1                      -> origin/$
>    * [new branch]      branch2                      -> origin/$
>    * [new branch]      some-really-long-branch-name -> origin/$
>    + 1234abc..5678def  branch3                      -> origin/$ (forced
>     update)
>    * [new branch]      branch4                      -> origin/$
>
> I've shrunk it a bit to fit in the email; my actual "long" name was much
> larger. And the average length for the shorter ones is, too, but the
> overall effect is the same; almost every line has a huge run of
> whitespace. And some lines wrap that would not have even under the
> normal, duplicated scheme.

I was about to resend with s/\$/*/g and ignored this issue (with a note) then it occurred to me we can simply ignore these long lines from column width calculation. Yeah such a line may still be wrapped around, but it will not push every other line to the far right. We already have code for that in adjust_refcol_width()

        max    = term_columns();
        ...
        /*
        * rough estimation to see if the output line is too long and
        * should not be counted (we can't do precise calculation
        * anyway because we don't know if the error explanation part
        * will be printed in update_local_ref)
        */
        if (21 /* flag and summary */ + rlen + 4 /* -> */ + llen >= max)
                return;
        ...

we can limit max to, like, term_columns() / 2 (or 2/3 of term_columns). There's no perfect number, some people will still find the output ugly _often_. But hopefully the majority won't. What do you think?

-- 
Duy