threads / rfc / 3879

RFC patchpager: do not fork a pager if environment variable PAGER is set to NONE

Subject: [RFC/PATCH] pager: do not fork a pager if environment variable PAGER is set to NONE

## tl;dr

5 messages between Apr 15, 2006 and Apr 16, 2006. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Johannes Schindelin· Apr 15, 2006, 13:16 UTC · lore
This helps debugging tremendously.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 pager.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
a91a9371e117555a03cfe7d24515787fb13005b6
Show changes to pager.c +4 −4
diff --git a/pager.c b/pager.c
index 1364e15..1136adf 100644
--- a/pager.c
+++ b/pager.c
@@ -5,9 +5,8 @@ #include "cache.h"
  * something different on Windows, for example.
  */
 
-static void run_pager(void)
+static void run_pager(const char *prog)
 {
-	const char *prog = getenv("PAGER");
 	if (!prog)
 		prog = "less";
 	setenv("LESS", "-S", 0);
@@ -16,10 +15,11 @@ static void run_pager(void)
 
 void setup_pager(void)
 {
+	const char *prog = getenv("PAGER");
 	pid_t pid;
 	int fd[2];
 
-	if (!isatty(1))
+	if (!isatty(1) || (prog != NULL && !strcmp(prog, "NONE")))
 		return;
 	if (pipe(fd) < 0)
 		return;
@@ -43,6 +43,6 @@ void setup_pager(void)
 	close(fd[0]);
 	close(fd[1]);
 
-	run_pager();
+	run_pager(prog);
 	exit(255);
 }

-- 
1.3.0.rc4.g667c
Junio C Hamano· Apr 15, 2006, 20:50 UTC · re: Johannes Schindelin · lore

Re: [RFC/PATCH] pager: do not fork a pager if environment variable PAGER is set to NONE

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> This helps debugging tremendously.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

I like what this wants to do. I am not so sure PAGER=NONE is a good convention, however.

Johannes Schindelin· Apr 16, 2006, 02:01 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH] pager: do not fork a pager if environment variable PAGER is set to NONE

Hi,
On Sat, 15 Apr 2006, Junio C Hamano wrote:
Show 8 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > This helps debugging tremendously.
> >
> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> 
> I like what this wants to do.  I am not so sure PAGER=NONE is a
> good convention, however.
I am sure it is not.

One solution would be to introduce yet another command line option "--no-pager", but I find that ugly.

Another solution would be to check if the environment variable NO_PAGER is set.

Wishes?

Ciao, Dscho

Junio C Hamano· Apr 16, 2006, 02:20 UTC · re: Johannes Schindelin · lore

Re: [RFC/PATCH] pager: do not fork a pager if environment variable PAGER is set to NONE

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 22 quoted lines
> Hi,
>
> On Sat, 15 Apr 2006, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > This helps debugging tremendously.
>> >
>> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>> 
>> I like what this wants to do.  I am not so sure PAGER=NONE is a
>> good convention, however.
>
> I am sure it is not.
>
> One solution would be to introduce yet another command line option 
> "--no-pager", but I find that ugly.
>
> Another solution would be to check if the environment variable NO_PAGER is 
> set.
>
> Wishes?

Honestly, undecided. Not that I think people would do PAGER=NONE to mean a program /usr/bin/NONE.

Right now, the following does not say anything:
	$ PAGER= git log

but I do not think that is a reasonably behaviour, either. So detecting PAGER is set to an empty string (not "nonexistence" - the current behaviour of defaulting to "less" in such a case is reasonable) is probably a better alternative.

A somewhat related topic; I often set PAGER=cat when I do not want the --[More]-- prompt and I thing many Emacs users do this. It might also be good to detect it and omit piping in such a case, but that is independent, so if you are going to do this as well, please make it a separate patch.

Johannes Schindelin· Apr 16, 2006, 02:44 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH] pager: do not fork a pager if environment variable PAGER is set to NONE

Hi,
On Sat, 15 Apr 2006, Junio C Hamano wrote:
Show 5 quoted lines
> A somewhat related topic; I often set PAGER=cat when I do not
> want the --[More]-- prompt and I thing many Emacs users do this.
> It might also be good to detect it and omit piping in such a
> case, but that is independent, so if you are going to do this as
> well, please make it a separate patch.

I was not quite sure if PAGER=cat could be taken as "user does not want any pager to be fork()ed", but you are probably right: PAGER=cat means that stdout is forwarded to stdout, i.e. we are better off not fork()ing and calling "cat":

--- [PATCH] pager: do not fork a pager if PAGER=cat

This helps debugging tremendously.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 pager.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
4a617a436bba9288cec5d3918d02c5b0e652df98
Show changes to pager.c +4 −4
diff --git a/pager.c b/pager.c
index 1364e15..86e93b3 100644
--- a/pager.c
+++ b/pager.c
@@ -5,9 +5,8 @@ #include "cache.h"
  * something different on Windows, for example.
  */
 
-static void run_pager(void)
+static void run_pager(const char *prog)
 {
-	const char *prog = getenv("PAGER");
 	if (!prog)
 		prog = "less";
 	setenv("LESS", "-S", 0);
@@ -16,10 +15,11 @@ static void run_pager(void)
 
 void setup_pager(void)
 {
+	const char *prog = getenv("PAGER");
 	pid_t pid;
 	int fd[2];
 
-	if (!isatty(1))
+	if (!isatty(1) || (prog != NULL && !strcmp(prog, "cat")))
 		return;
 	if (pipe(fd) < 0)
 		return;
@@ -43,6 +43,6 @@ void setup_pager(void)
 	close(fd[0]);
 	close(fd[1]);
 
-	run_pager();
+	run_pager(prog);
 	exit(255);
 }

-- 
1.3.0.rc4.gb6b20-dirty

← back to recent threads