git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rubén Justo" <rjusto@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: [PATCH 3/4] pager: introduce wait_for_pager
Date: Thu, 25 Jul 2024 15:44:39 +0200	[thread overview]
Message-ID: <934b8247-a5e7-4919-8ccb-08ceb23c03ff@gmail.com> (raw)
In-Reply-To: <76936fb1-446d-455f-b4e7-6e24dda3c17d@gmail.com>

Since f67b45f862 (Introduce trivial new pager.c helper infrastructure,
2006-02-28) we have the machinery to send our output to a pager.

That machinery, once set up, does not allow us to regain the original
stdio streams.

In the interactive commands (i.e.: add -p) we want to use the pager for
some output, while maintaining the interaction with the user.

Modify the pager machinery so that we can use `setup_pager()` and, once
we've finished sending the desired output for the pager, wait for the
pager termination using a new function `wait_for_pager()`.  Make this
function reset the pager machinery before returning.

One specific point to note is that we avoid forking the pager in
`setup_pager()` if the configured pager is an empty string [*1*] or
simply "cat" [*2*].  In these cases, `setup_pager()` does nothing and
therefore `wait_for_pager()` should not be called.

We could modify `setup_pager()` to return an indication of these
situations, so we could avoid calling `wait_for_pager()`.

However, let's avoid transferring 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 the pager.

   1.- 402461aab1 (pager: do not fork a pager if PAGER is set to empty.,
                   2006-04-16)

   2.- caef71a535 (Do not fork PAGER=cat, 2006-04-16)

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
 pager.c | 46 ++++++++++++++++++++++++++++++++++++++++------
 pager.h |  1 +
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/pager.c b/pager.c
index 251adfc2ad..896f40fcd2 100644
--- a/pager.c
+++ b/pager.c
@@ -14,7 +14,7 @@ int pager_use_color = 1;
 
 static struct child_process pager_process;
 static char *pager_program;
-static int close_fd2;
+static int old_fd1 = -1, old_fd2 = -1;
 
 /* Is the value coming back from term_columns() just a guess? */
 static int term_columns_guessed;
@@ -24,11 +24,11 @@ static void close_pager_fds(void)
 {
 	/* signal EOF to pager */
 	close(1);
-	if (close_fd2)
+	if (old_fd2 != -1)
 		close(2);
 }
 
-static void wait_for_pager_atexit(void)
+static void finish_pager(void)
 {
 	fflush(stdout);
 	fflush(stderr);
@@ -36,8 +36,37 @@ static void wait_for_pager_atexit(void)
 	finish_command(&pager_process);
 }
 
+static void wait_for_pager_atexit(void)
+{
+	if (old_fd1 == -1)
+		return;
+
+	finish_pager();
+}
+
+void wait_for_pager(void)
+{
+	if (old_fd1 == -1)
+		return;
+
+	finish_pager();
+	sigchain_pop_common();
+	unsetenv("GIT_PAGER_IN_USE");
+	dup2(old_fd1, 1);
+	close(old_fd1);
+	old_fd1 = -1;
+	if (old_fd2 != -1) {
+		dup2(old_fd2, 2);
+		close(old_fd2);
+		old_fd2 = -1;
+	}
+}
+
 static void wait_for_pager_signal(int signo)
 {
+	if (old_fd1 == -1)
+		return;
+
 	close_pager_fds();
 	finish_command_in_signal(&pager_process);
 	sigchain_pop(signo);
@@ -113,6 +142,7 @@ void prepare_pager_args(struct child_process *pager_process, const char *pager)
 
 void setup_pager(void)
 {
+	static int once = 0;
 	const char *pager = git_pager(isatty(1));
 
 	if (!pager)
@@ -142,16 +172,20 @@ void setup_pager(void)
 		die("unable to execute pager '%s'", pager);
 
 	/* original process continues, but writes to the pipe */
+	old_fd1 = dup(1);
 	dup2(pager_process.in, 1);
 	if (isatty(2)) {
-		close_fd2 = 1;
+		old_fd2 = dup(2);
 		dup2(pager_process.in, 2);
 	}
 	close(pager_process.in);
 
-	/* this makes sure that the parent terminates after the pager */
 	sigchain_push_common(wait_for_pager_signal);
-	atexit(wait_for_pager_atexit);
+
+	if (!once) {
+		once++;
+		atexit(wait_for_pager_atexit);
+	}
 }
 
 int pager_in_use(void)
diff --git a/pager.h b/pager.h
index b77433026d..103ecac476 100644
--- a/pager.h
+++ b/pager.h
@@ -5,6 +5,7 @@ struct child_process;
 
 const char *git_pager(int stdout_is_tty);
 void setup_pager(void);
+void wait_for_pager(void);
 int pager_in_use(void);
 int term_columns(void);
 void term_clear_line(void);
-- 
2.46.0.rc0.4.g6f4990c0d4

  parent reply	other threads:[~2024-07-25 13:44 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-23  0:39 [PATCH v2 0/2] add-p P fixups Rubén Justo
2024-07-23  0:42 ` [PATCH v2 1/2] t3701: avoid one-shot export for shell functions Rubén Justo
2024-07-23  0:42 ` [PATCH v2 2/2] pager: make wait_for_pager a no-op for "cat" Rubén Justo
2024-07-23  0:54 ` [PATCH v2 0/2] add-p P fixups Junio C Hamano
2024-07-23  9:15 ` Phillip Wood
2024-07-23 16:52   ` Junio C Hamano
2024-07-23 22:08   ` Rubén Justo
2024-07-24 15:21     ` phillip.wood123
2024-07-24 16:12       ` Rubén Justo
2024-07-25  9:45         ` Phillip Wood
2024-07-25 12:16           ` Rubén Justo
2024-07-25 13:42             ` [PATCH 0/4] squash fixups in rj/add-p-pager Rubén Justo
2024-07-25 13:44               ` [PATCH 1/4] add-patch: test for 'p' command Rubén Justo
2024-07-25 13:44               ` [PATCH 2/4] pager: do not close fd 2 unnecessarily Rubén Justo
2024-07-25 13:44               ` Rubén Justo [this message]
2024-07-25 13:44               ` [PATCH 4/4] add-patch: render hunks through the pager Rubén Justo
2024-07-25 15:24           ` [PATCH v2 0/2] add-p P fixups Junio C Hamano
2024-07-25 16:41             ` Re* " Rubén Justo
2024-07-25 16:43               ` [PATCH 1/2] pager: introduce wait_for_pager Rubén Justo
2024-07-25 16:43               ` [PATCH 2/2] add-patch: render hunks through the pager Rubén Justo
2024-07-26 18:36                 ` Junio C Hamano
2024-07-27  1:40                   ` Junio C Hamano
2024-07-27 14:33                     ` Rubén Justo
2024-07-26 18:24               ` Re* [PATCH v2 0/2] add-p P fixups Junio C Hamano
2024-07-26 19:22                 ` Rubén Justo
2024-07-26 19:48                   ` Junio C Hamano
2024-07-26 20:16                     ` Rubén Justo
2024-07-28  9:11                       ` Rubén Justo
2024-07-29 18:45                         ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2024-07-12  0:57 [PATCH 0/4] use the pager in 'add -p' Rubén Justo
2024-07-12  1:00 ` [PATCH 3/4] pager: introduce wait_for_pager Rubén Justo
2024-07-12 13:17   ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=934b8247-a5e7-4919-8ccb-08ceb23c03ff@gmail.com \
    --to=rjusto@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).