From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 01 Aug 2016 18:07:37 GMT
Subject: Re: [PATCH 1/2] pager: move pager-specific setup into the build
Message-ID: <xmqq37mol5w6.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160801172459.wz7wigmytxpfvs22@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I'm not too worried about spaces here. This is a resurrection of an old
> discussion, and in all that time, I think the only realistic suggestions
> for built-in values have been pretty tame.
>
> If this were used to parse arbitrary user-provided runtime values, I'd
> be more concerned. But I'm not sure why we would need that. Your $EDITOR
> example is arbitrary shell code, and we let the shell handle it (modulo
> some efficiency shortcuts). Likewise, fancy runtime things should go in
> GIT_PAGER, where you can not only set options with spaces, but do fancy
> things like pipes, shell functions, etc.
>
> The use of stringify() here is funny to me; I think there is a cpp
> tokenizing step in the middle that will do things like gobble up
> whitespace (but I'm not sure if it has other possible effects). I think
> our more usual method here would be to C-quote in the Makefile (with the
> equivalent of 's/\\/\\\\/g; s/"/\\"/g'), and then pass it to the
> compiler as a string literal, like -DPAGER_ENV=\"$(PAGER_ENV_CQ_SQ\".

All sensible arguments, including the rationale to reject 2/2.

Thanks.