[Quilt-dev] Re: [PATCH] pager support like git

[Bert Wesarg]

On Wed, Nov 18, 2009 at 17:41, Andreas Gruenbacher <address@hidden> wrote:
> Bert,
>
> I like this feature. There are a few little remaining problems though.
>
>> diff --git a/quilt/diff.in b/quilt/diff.in
>> index 8435024..866d61c 100644
>> --- a/quilt/diff.in
>> +++ b/quilt/diff.in
>> @@ -196,7 +196,7 @@ do
>>                       opt_color=1 ;;
>>               auto | tty)
>>                       opt_color=
>> -                     [ -t 1 ] && opt_color=1 ;;
>> +                     [ -t 1 -o -n "${QUILT_PAGER_IN_USE-}" ] && opt_color=1 
>> ;;
>>               never)
>>                       opt_color= ;;
>>               *)
>> @@ -310,6 +310,8 @@ then
>>       || die 1
>>  fi
>>
>> +setup_pager
>> +
>>  for file in "address@hidden"
>>  do
>>       if [ -n "$opt_snapshot" -a -e "$QUILT_PC/$snap_subdir/$file" ]
>
> Here and in some other places, setup_pager is called after evaluating
> QUILT_PAGER_IN_USE though. This looks wrong.

As a first proposal to solve this, we can defer the auto color
detection after the pager may have started. Like this:

in getopt loop:

                auto | tty)
                        # defer auto color after pager may have started
                        opt_color=auto

:

setup_pager
if [ "x$opt_color" = xauto ]
then
        opt_color=
        [ -t 1 -o -n "${QUILT_PAGER_IN_USE-}" ] && opt_color=1 ;;
fi

As a second proposal, we could pass the value of --color to the
setup_pager function and put the logic in there. This sounds actually
nicer, does it?

Bert