git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] pager: introduce wait_for_pager

From
Rubén Justo <rjusto@gmail.com>
Date
Jul 25, 2024, 16:43 UTC
Message-ID
<2ec2a894-a173-469c-8211-ac5f49a82a27@gmail.com>
In-Reply-To
<24e83a0f-b0c8-4cd5-b321-1d7702b844ce@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
Previous: Rubén JustoNext: Rubén Justo
Message 19 of 29 in “add-p P fixups”
  1. 0/2 add-p P fixupsRubén Justo, Jul 23, 2024
  2. 1/2 t3701: avoid one-shot export for shell functionsRubén Justo, Jul 23, 2024
  3. 2/2 pager: make wait_for_pager a no-op for "cat"Rubén Justo, Jul 23, 2024
  4. Junio C HamanoJul 23, 2024
  5. Phillip WoodJul 23, 2024
  6. Junio C HamanoJul 23, 2024
  7. Rubén JustoJul 23, 2024
  8. phillip.wood123@gmail.comJul 24, 2024
  9. Rubén JustoJul 24, 2024
  10. Phillip WoodJul 25, 2024
  11. Rubén JustoJul 25, 2024
  12. 0/4 squash fixups in rj/add-p-pagerRubén Justo, Jul 25, 2024
  13. 1/4 add-patch: test for 'p' commandRubén Justo, Jul 25, 2024
  14. 2/4 pager: do not close fd 2 unnecessarilyRubén Justo, Jul 25, 2024
  15. 3/4 pager: introduce wait_for_pagerRubén Justo, Jul 25, 2024
  16. 4/4 add-patch: render hunks through the pagerRubén Justo, Jul 25, 2024
  17. Junio C HamanoJul 25, 2024
  18. Re* [PATCH v2 0/2] add-p P fixupsRubén Justo, Jul 25, 2024
  19. 1/2 pager: introduce wait_for_pagerRubén Justo, Jul 25, 2024
  20. 2/2 add-patch: render hunks through the pagerRubén Justo, Jul 25, 2024
  21. Junio C HamanoJul 26, 2024
  22. Junio C HamanoJul 27, 2024
  23. Rubén JustoJul 27, 2024
  24. Junio C HamanoJul 26, 2024
  25. Rubén JustoJul 26, 2024
  26. Junio C HamanoJul 26, 2024
  27. Rubén JustoJul 26, 2024
  28. Rubén JustoJul 28, 2024
  29. Junio C HamanoJul 29, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.