Thread (1 message) 1 message, 1 author, 2016-08-01

Re: [PATCH 1/2] pager: move pager-specific setup into the build

From: Junio C Hamano <hidden>
Date: 2016-08-01 18:15:30

Jeff King [off-list ref] 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help