Re: [PATCH v2 0/2] add-p P fixups

> On Tue, Jul 23, 2024 at 10:15:03AM +0100, Phillip Wood wrote:
> 
>> As rj/add-p-pager is only in seen I
>> assume you'll re-roll with these squashed in once everyone is happy?
> 
> Junio has already integrated these changes into the branch he has in his
> tree, including a small change to the message to adjust it to his
> comments, which I think is good.
> 
> I hope that what we already have in Junio's tree is the final iteration
> of this long series and that we can let it settle before making further
> changes.

>>
>> Best Wishes
>>
>> Phillip
>>
>> On 23/07/2024 01:39, Rubén Justo wrote:
>>> Rubén Justo (1):
>>>     t3701: avoid one-shot export for shell functions
>>>     pager: make wait_for_pager a no-op for "cat"
>>>
>>>    pager.c                    | 3 +++
>>>    t/t3701-add-interactive.sh | 6 +++++-
>>>    2 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> Range-diff against v1:
>>> 1:  c3b8ebbae7 ! 1:  15fbf82fff t3701: avoid one-shot export for shell functions
>>>       @@ Commit message
>>>                VAR=VAL command args
>>>       -    it's a common way to define one-shot variables within the scope of
>>>       +    is a common way to set and export one-shot variables within the scope of
>>>            executing a "command".
>>>            However, when "command" is a function which in turn executes the
>>>       @@ Commit message
>>>                $ A=1 f
>>>                A=
>>>       +    Note that POSIX is not specific about this behavior:
>>>       +
>>>       +    http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01
>>>       +
>>>            One of our CI jobs on GitHub Actions uses Ubuntu 20.04 running dash
>>>            0.5.10.2-6, so we failed the test t3701:51;  the "git add -p" being
>>>            tested did not get our custom GIT_PAGER, which broke the test.
>>> 2:  f45455f1ff ! 2:  b87c3d96e4 pager: make wait_for_pager a no-op for "cat"
>>>       @@ Commit message
>>>            "cat" [*2*], then we return from `setup_pager()` silently without doing
>>>            anything, allowing the output to go directly to the normal stdout.
>>>       -    Let's make the call to `wait_for_pager()` for these cases, or any other
>>>       -    future optimizations that may occur, also exit silently without doing
>>>       -    anything.
>>>       +    If `setup_pager()` avoids forking a pager, then when the client calls
>>>       +    the corresponding `wait_for_pager()`, we might fail trying to terminate
>>>       +    a process that wasn't started.
>>>       +
>>>       +    One solution to avoid this problem could be to make the caller aware
>>>       +    that `setup_pager()` did nothing, so it could avoid calling
>>>       +    `wait_for_pager()`.
>>>       +
>>>       +    However, let's avoid shifting that responsibility to the caller and
>>>       +    instead treat the call to `wait_for_pager()` as a no-op when we know we
>>>       +    haven't forked a pager.
>>>               1.- 402461aab1 (pager: do not fork a pager if PAGER is set to empty.,
>>>                               2006-04-16)