Re: ab/* topics (was: Re: What's cooking in git.git (Nov 2018, #01; Thu, 1))

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Nov 1, 2018, 14:30 UTC
Message-ID: <20181101143006.GV30222@szeder.dev>
In-Reply-To: <87va5gkj0e.fsf@evledraar.gmail.com>

On Thu, Nov 01, 2018 at 02:46:41PM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 12 quoted lines

> > However, if you push that patch with 'sh-i18n--helper' as-is, then I
> > do object now: parsing a boolean in shell is not at all that difficult
> > to justify this new command.
> 
> So instead of calling a helper (which is undocumented, and only used by
> git itself internally) you'd instead like to have some shellscript
> thingy like:
> 
>     if test $value = 'true' -o $value = '1' [....]
>     then
>         exit 0
>     elif test $value = 'false' -o $value = '0' [...]

Yes, but more like:

  case "$GIT_TEST_GETTEXT_POISON" in
  yes|true|on|1)
    GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
    ;;
  esac

There is no need to check for 'false', 0, etc.

Yes, I know that this is not as fully-fledged as git_env_bool(), e.g. it won't recognize 'TrUe' and '42' as true, but since this is "merely" a testing aid, I think it's sufficient.

> Sure, if that's the consensus I can change that, but it seems like the
> opposite of the direction we're moving with the general *.sh -> *.c
> migration. I.e. implementing helpers whenever possible instead of adding
> new shellscript-only logic.

I doubt that we really want to implement helpers "whenever possible", i.e. even instead of such a rather trivial piece of shell code.

In the discusson of v1 of that patch there were suggestions on how to turn an environment variable into a boolean in a more proper way, e.g. by extending 'git var', but I agree with Peff that "we should do nothing for now and see how often this comes up" [1]. In the meantime this builtin seems to be the worse direction of all.

[1] https://public-inbox.org/git/20181025212358.GA23257@sigill.intra.peff.net/