Why does git-mergetool use /dev/tty?

git-mergetool.sh, lines 298-302:

    if test $last_status -ne 0; then
        prompt_after_failed_merge < /dev/tty || exit 1
    fi
    printf "\n"
    merge_file "$i" < /dev/tty > /dev/tty

Why does git-mergetool ignore the provided STDIN and STDOUT when not given a path to merge?

This wasn't apparent until bb0a484: "mergetool: Skip autoresolved paths" added a test that doesn't provide a file to merge on the command line.

~~ Brian Gernhardt

Brian Gernhardt wrote:

Why does git-mergetool ignore the provided STDIN and STDOUT when not given a path to merge?

See af314714 (mergetool: Remove explicit references to /dev/tty,
2010-08-20) in pu.

Hope that helps,
Jonathan

On Aug 23, 2010, at 9:53 PM, Jonathan Nieder wrote:

Brian Gernhardt wrote:

quoted

Why does git-mergetool ignore the provided STDIN and STDOUT when not given a path to merge?

See af314714 (mergetool: Remove explicit references to /dev/tty,
2010-08-20) in pu.

Hope that helps,

It does really help, actually.  One thing removed from my TODO.

~~ Brian

On Mon, Aug 23, 2010 at 09:49:57PM -0400, Brian Gernhardt wrote:

git-mergetool.sh, lines 298-302:

quoted

    if test $last_status -ne 0; then
        prompt_after_failed_merge < /dev/tty || exit 1
    fi
    printf "\n"
    merge_file "$i" < /dev/tty > /dev/tty

Why does git-mergetool ignore the provided STDIN and STDOUT when not given a path to merge?

It doesn't deliberately ignore them, it merely loses them because what
you've quoted is in the middle of a redirect, you snipped:

    files_to_merge |
    while IFS= read i
    do  

mergetool is designed to be an interactive tool and didn't originally
have any tests so this was a reasonable (if ugly) way to restore
access to the user for the merge_file function.

This is also why all the previous test provided an explict list of
files to merge, because this was the only testable way to invoke
mergetool.

There's a proposed patch to save and restore the original stdin
instead of assuming that there is a tty in pu: af314714.

If the current behaviour is causing an issue for you then testing this
fix would be appreciated.

Thanks,

Charles.

On Aug 24, 2010, at 2:43 AM, Charles Bailey wrote:

There's a proposed patch to save and restore the original stdin
instead of assuming that there is a tty in pu: af314714.

If the current behaviour is causing an issue for you then testing this
fix would be appreciated.

The issue was spurious output during tests, which is fixed by af314714.

Thanks for the explanation though, I had missed that it was part of a pipeline.

~~ Brian