Re: [PATCH v6] blame: add support for --[no-]progress option

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 13, 2015, 00:40 UTC
Message-ID: <CAPig+cQ__eueXdeq=CHUicz727hx-THuJxt20_PXGgo=KzK0xA@mail.gmail.com>
In-Reply-To: <CAOc6etYAbT8Yc3NwB4Rh1dShjDQB_PR2K-HvXpx6dTcB=YaB8w@mail.gmail.com>

On Sat, Dec 12, 2015 at 7:36 PM, Edmundo Carmona Antoranz <eantoranz@gmail.com> wrote:

Show 13 quoted lines

> On Sat, Dec 12, 2015 at 6:30 PM, Edmundo Carmona Antoranz
> <eantoranz@gmail.com> wrote:
> Hmmmm.... if the code in assign_blame changed to this, it would be
> possible to allow the -1 to go through:
>
> if (show_progress > 0)
>     pi.progress = start_progress_delay(_("Blaming lines"),
> sb->num_lines, 50, 1);
>
> But then I think it would be more 'concise' if we had the value set to
> 0/1 instead of expecting to see a possible value of -1 there (or
> anywhere else) after progressing if progress will be shown or not in
> the piece of code we are chatting about.

The name "show_progress" does read like a boolean rather than a tristate, so making sure its value is 0 or 1 after option processing (as your current patch does) is probably the best way to go. I don't otherwise feel strongly about it.