Re: [PATCH] mergetool: Remove explicit references to /dev/tty

From: Charles Bailey <charles@hashpling.org>
Date: Aug 20, 2010, 13:50 UTC
Message-ID: <4C6E883A.2030301@hashpling.org>
In-Reply-To: <20100820122724.GS10407@burratino>

On 20/08/2010 13:27, Jonathan Nieder wrote:

Show 12 quoted lines

>> +++ b/git-mergetool.sh
>> @@ -292,14 +292,15 @@ if test $# -eq 0 ; then
>>       printf "Merging:\n"
>>       printf "$files\n"
>>
>> -    files_to_merge |
>> +    # Save original stdin to fd 3
>> +    files_to_merge 3<&0 |
>
> I would think this should work, but it doesn't feel idiomatic.  Why
> not save stdin a little earlier, so the reader does not have to track
> down whether it has been redirected?

No special reason, I just thought it was more natural to save it at the time that we do the redirect..

> The test quietly passes for me with dash but fails with ksh:
>
>   /home/jrn/src/git4/git-mergetool: line 303: 3: cannot open [Bad file descriptor]

... but given that this approach is evidently less portable your way is clearly better.

> With the patch below on top, it passes with dash and ksh.

Thanks, I'll re-roll in a bit at squash your fixes in, if that's OK?

Charles.