Re: [PATCH] sha1_name.c: add an option to abort on ambiguous refs

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 23, 2016, 23:11 UTC
Message-ID: <CACsJy8D7B0wRiBqz6G3vrpLKUEGaoBvzf_a97R1LVU0UrKQ2rw@mail.gmail.com>
In-Reply-To: <alpine.DEB.2.20.1603231642580.4690@virtualbox>

On Wed, Mar 23, 2016 at 10:45 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 17 quoted lines

> Hi Duy,
>
> On Wed, 23 Mar 2016, Nguyễn Thái Ngọc Duy wrote:
>
>> There are cases when a warning on ambiguous refs may go unnoticed
>> (e.g. git-log filling up the whole screen). There are also cases when
>> people want to catch ambiguity early (e.g. it happens deep in some
>> script). In either case, aborting the program would accomplish it.
>
> Whenever I see a die() in code outside of builtin/*.c, I cringe. I do that
> because it was *exactly* something like that that caused a serious
> regression in the builtin am so that we had to resort back to spawning
> separate processes *just so* that we could catch error conditions and
> run certain code in that case.
>
> Maybe there would be a way to do what you want to do that does not fly in
> the face of libification?

Sorry I got nothing better.

> Maybe some strbuf with an atexit() that
> accumulates fatal errors that might be hidden and that are then written at
> the end of the program (colorfully, if isatty(2))?

That sounds hacky. If you don't want do die() deep inside then the callers have to handle it, but get_sha1() is spread everywhere. Plus I do not want to program to succeed when there is ambiguation because the ref git chooses may not be what I want. I want it to abort (and almost made a patch that die() unconditionally).

-- 
Duy