[PATCH stable] pager: exec pager using /bin/sh -c

[Dan Villiom Podlaski Christiansen]

On 21 May 2010, at 15:05, Augie Fackler wrote:

> 
> On May 21, 2010, at 8:03 AM, Dan Villiom Podlaski Christiansen wrote:
> 
>> On 21 May 2010, at 14:16, Martin Geisler wrote:
>> 
>>> Augie Fackler <durin42 at gmail.com> writes:
>>> 
>>>> On May 21, 2010, at 7:04 AM, Martin Geisler wrote:
>>>> 
>>>>> Brodie Rao <brodie at bitheap.org> writes:
>>>>> 
>>>>>> # HG changeset patch
>>>>>> # User Brodie Rao <brodie at bitheap.org>
>>>>>> # Date 1274280852 18000
>>>>>> # Branch stable
>>>>>> # Node ID 9c655d72da80c45a843e213c247003860cb25f8b
>>>>>> # Parent  b5c0f6a1143066c453032ab6dc4163ef52e932ee
>>>>>> pager: exec pager using /bin/sh -c
>>>>>> 
>>>>>> This mimics the behavior of popen() and ensures setting the pager to
>>>>>> something like "LESS=FSRX less" will work.
>>>>> 
>>>>> I don't understand why people would do that when they can just as well
>>>>> use command line options, or they could export the LESS variable in
>>>>> their shell startup file.
>>>> 
>>>> Trouble is even using CLI options requires either splitting arguments
>>>> (potentially dangerous, easy to screw up) or using system()/popen().
>>>> The latter is an option, but the docs even suggested setting the pager
>>>> to "LESS=FSRX less" IIRC.
>>> 
>>> Yeah, and I was looking at the docs and thinking "huh?" :-)
>>> 
>>> Maybe it's just me who has never considered setting an environment
>>> option for something like this where a command line flag or shell alias
>>> works just as well.
>> 
>> Well, users might have an environment variable set which they might want to override. For example, the LESS environment variable. Regardless, this is behaviour we have supported in the past.
>> 
>> The patch has one minor bug, though: The user's shell should be used rather than hardcoding ‘/bin/sh’. It's easily fixed:
> 
> No, the patch is correct: http://linux.die.net/man/3/system

Right, even <http://www.opengroup.org/onlinepubs/009695399/functions/system.html> agrees. (I didn't know that…) Never mind that comment, but I'd still very like to see this patch get in :)

--

Dan Villiom Podlaski Christiansen
danchr at gmail.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 1943 bytes
Desc: not available
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20100521/93173db2/attachment.bin>