Hi Rubén
These changes themselves look sensible. As rj/add-p-pager is only in
seen I assume you'll re-roll with these squashed in once everyone is happy?
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)
