{"thread":{"id":"12139","subject":"[RFC] sending errors to stdout under $PAGER","startedAt":"2008-02-16T19:15:41Z","lastAt":"2008-02-17T19:38:23Z","messageCount":9,"participants":["Junio C Hamano","Shawn O. Pearce","Jeff King","Edgar Toernig","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"68919","messageId":"7vbq6g90gy.fsf@gitster.siamese.dyndns.org","threadId":"12139","inReplyTo":null,"subject":"[RFC] sending errors to stdout under $PAGER","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-16T19:15:41Z","receivedAt":"2008-02-16T19:15:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"If you do this (and you are not an Emacs user who uses PAGER=cat\nin your *shell* buffer):\n\n        $ git init\n        Initialized empty Git repository in .git/\n        $ echo hello world >foo\n        $ H=$(git hash-object -w foo)\n        $ git tag -a foo-tag -m \"Tags $H\" $H\n        $ echo $H\n        3b18e512dba79e4c8300dd08aeb37f8e728b8dad\n        $ rm -f .git/objects/3b/18e5*\n        $ git show foo-tag\n        tag foo-tag\n        Tagger: Junio C Hamano <gitster@pobox.com>\n        Date:   Sat Feb 16 10:43:23 2008 -0800\n\n        Tags 3b18e512dba79e4c8300dd08aeb37f8e728b8dad\n\nyou do not get any indication of error.  If you are careful, you\nwould notice that no contents from the tagged object is\ndisplayed, but that is about it.  If you run the \"show\" command\nwithout pager, however, you will see the error:\n\n        $ git --no-pager show foo-tag\n        tag foo-tag\n        Tagger: Junio C Hamano <gitster@pobox.com>\n        Date:   Sat Feb 16 10:43:23 2008 -0800\n\n        Tags 3b18e512dba79e4c8300dd08aeb37f8e728b8dad\n        error: Could not read object 3b18e512dba79e4c8300dd08aeb37f8e728b8dad\n\nBecause we spawn the pager as the foreground process and feed\nits input via pipe from the real command, we cannot affect the\nexit status the shell sees from git command when the pager is in\nuse (I think there is not much gain we can have by working it\naround, though).  But at least it may make sense to show the\nerror message to the user sitting in front of the pager, perhaps\nlike this.\n\nWhat do people think?  Have I overlooked any downsides?\n\n---\n usage.c |    5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/usage.c b/usage.c\nindex a5fc4ec..681b84a 100644\n--- a/usage.c\n+++ b/usage.c\n@@ -4,12 +4,15 @@\n  * Copyright (C) Linus Torvalds, 2005\n  */\n #include \"git-compat-util.h\"\n+#include \"cache.h\"\n \n static void report(const char *prefix, const char *err, va_list params)\n {\n \tchar msg[256];\n+\tFILE *outto = (pager_in_use() ? stdout : stderr);\n+\n \tvsnprintf(msg, sizeof(msg), err, params);\n-\tfprintf(stderr, \"%s%s\\n\", prefix, msg);\n+\tfprintf(outto, \"%s%s\\n\", prefix, msg);\n }\n \n static NORETURN void usage_builtin(const char *err)\n"},{"id":"68956","messageId":"20080217075033.GP24004@spearce.org","threadId":"12139","inReplyTo":"7vbq6g90gy.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC] sending errors to stdout under $PAGER","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-02-17T07:50:33Z","receivedAt":"2008-02-17T07:50:33Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> Because we spawn the pager as the foreground process and feed\n> its input via pipe from the real command, we cannot affect the\n> exit status the shell sees from git command when the pager is in\n> use (I think there is not much gain we can have by working it\n> around, though).  But at least it may make sense to show the\n> error message to the user sitting in front of the pager, perhaps\n> like this.\n> \n> What do people think?  Have I overlooked any downsides?\n\nI think this is a good idea.\n\nIf you are using an interactive pager, you have asked for the content\nto come to you through that.  Not unlike how I have chosen to have\nthe content come to me through a virtual pty and not a printer with\ngreen-and-white bar paper.  :)\n\nI've been bitten by this in the past a few times, but I have also\nbeen knowledgable enough about git, the command I ran, and the\nproject I ran it on to realize something wasn't right with the\noutput I am seeing in the pager and retry without the pager to see\nthe real error(s).\n\n> +++ b/usage.c\n> @@ -4,12 +4,15 @@\n>   * Copyright (C) Linus Torvalds, 2005\n>   */\n>  #include \"git-compat-util.h\"\n> +#include \"cache.h\"\n>  \n>  static void report(const char *prefix, const char *err, va_list params)\n>  {\n>  \tchar msg[256];\n> +\tFILE *outto = (pager_in_use() ? stdout : stderr);\n> +\n>  \tvsnprintf(msg, sizeof(msg), err, params);\n> -\tfprintf(stderr, \"%s%s\\n\", prefix, msg);\n> +\tfprintf(outto, \"%s%s\\n\", prefix, msg);\n>  }\n\n-- \nShawn.\n"},{"id":"68980","messageId":"20080217123538.GB26031@sigill.intra.peff.net","threadId":"12139","inReplyTo":"7vbq6g90gy.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC] sending errors to stdout under $PAGER","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-17T12:35:38Z","receivedAt":"2008-02-17T12:35:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 16, 2008 at 11:15:41AM -0800, Junio C Hamano wrote:\n\n> Because we spawn the pager as the foreground process and feed\n> its input via pipe from the real command, we cannot affect the\n> exit status the shell sees from git command when the pager is in\n> use (I think there is not much gain we can have by working it\n> around, though).  But at least it may make sense to show the\n> error message to the user sitting in front of the pager, perhaps\n> like this.\n> \n> What do people think?  Have I overlooked any downsides?\n\nI think this makes sense. It could be annoying if chatty stderr output\ngot mixed in with the actual output, making things harder to read. But\ngit is not very chatty in general, and the point is that things sent to\nstderr _should_ grab the user's attention.\n\nThe only downside I see is that it disrupts the parsing of the output.\nIn most cases, this doesn't matter, since anything parsing the output\nwill disable the pager. The notable exception is something like 'tig',\nwhich I believe can act as a git pager which understands the output; it\ncan potentially be confused by the extra lines on stdout.\n\n-Peff\n"},{"id":"68988","messageId":"20080217144854.56fcb98d.froese@gmx.de","threadId":"12139","inReplyTo":"7vbq6g90gy.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC] sending errors to stdout under $PAGER","fromName":"Edgar Toernig","fromEmail":"froese@gmx.de","sentAt":"2008-02-17T13:48:54Z","receivedAt":"2008-02-17T13:48:54Z","isPatch":false,"sender":{"key":"froese@gmx.de","avatar":null},"body":"Junio C Hamano wrote:\n>\n> +\tFILE *outto = (pager_in_use() ? stdout : stderr);\n> +\n>  \tvsnprintf(msg, sizeof(msg), err, params);\n> -\tfprintf(stderr, \"%s%s\\n\", prefix, msg);\n> +\tfprintf(outto, \"%s%s\\n\", prefix, msg);\n>\n> What do people think?  Have I overlooked any downsides?\n\nWouldn't it be better/safer to redirect stderr to the pager\nin the first place?\n\nSo, instead of the current\n\n\tfoo | less\nuse\n\tfoo 2>&1 | less\n\nor, in pager.c:\n\n         /* return in the child */\n        if (!pid) {\n                dup2(fd[1], 1);\n+               dup2(fd[1], 2);\n                close(fd[0]);\n                close(fd[1]);\n                return;\n        }\n\nCiao, ET.\n"},{"id":"68993","messageId":"alpine.LSU.1.00.0802171515190.30505@racer.site","threadId":"12139","inReplyTo":"20080217144854.56fcb98d.froese@gmx.de","subject":"Re: [RFC] sending errors to stdout under $PAGER","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-17T15:15:43Z","receivedAt":"2008-02-17T15:15:43Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 17 Feb 2008, Edgar Toernig wrote:\n\n> Junio C Hamano wrote:\n> >\n> > +\tFILE *outto = (pager_in_use() ? stdout : stderr);\n> > +\n> >  \tvsnprintf(msg, sizeof(msg), err, params);\n> > -\tfprintf(stderr, \"%s%s\\n\", prefix, msg);\n> > +\tfprintf(outto, \"%s%s\\n\", prefix, msg);\n> >\n> > What do people think?  Have I overlooked any downsides?\n> \n> Wouldn't it be better/safer to redirect stderr to the pager\n> in the first place?\n> \n> [...]\n>\n>          /* return in the child */\n>         if (!pid) {\n>                 dup2(fd[1], 1);\n> +               dup2(fd[1], 2);\n>                 close(fd[0]);\n>                 close(fd[1]);\n>                 return;\n>         }\n\nI like it.\n\nCiao,\nDscho\n"},{"id":"69005","messageId":"7vd4qv1n78.fsf@gitster.siamese.dyndns.org","threadId":"12139","inReplyTo":"20080217144854.56fcb98d.froese@gmx.de","subject":"Re: [RFC] sending errors to stdout under $PAGER","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-17T17:56:27Z","receivedAt":"2008-02-17T17:56:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edgar Toernig <froese@gmx.de> writes:\n\n> Junio C Hamano wrote:\n>>\n>> +\tFILE *outto = (pager_in_use() ? stdout : stderr);\n>> +\n>>  \tvsnprintf(msg, sizeof(msg), err, params);\n>> -\tfprintf(stderr, \"%s%s\\n\", prefix, msg);\n>> +\tfprintf(outto, \"%s%s\\n\", prefix, msg);\n>>\n>> What do people think?  Have I overlooked any downsides?\n>\n> Wouldn't it be better/safer to redirect stderr to the pager\n> in the first place?\n>\n> So, instead of the current\n>\n> \tfoo | less\n> use\n> \tfoo 2>&1 | less\n\nI like it.  Much nicer.  Thanks.\n"},{"id":"69008","messageId":"20080217181523.GA4818@coredump.intra.peff.net","threadId":"12139","inReplyTo":"7vd4qv1n78.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC] sending errors to stdout under $PAGER","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-17T18:15:23Z","receivedAt":"2008-02-17T18:15:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 17, 2008 at 09:56:27AM -0800, Junio C Hamano wrote:\n\n> > Wouldn't it be better/safer to redirect stderr to the pager\n> > in the first place?\n> >\n> > So, instead of the current\n> >\n> > \tfoo | less\n> > use\n> > \tfoo 2>&1 | less\n> \n> I like it.  Much nicer.  Thanks.\n\nThis will also put the stderr of any sub-programs into the pager, which\nis probably worse if you have, e.g., a chatty external diff program. I\ndon't know if we care enough about that.\n\n-Peff\n"},{"id":"69011","messageId":"7vr6fbzbky.fsf@gitster.siamese.dyndns.org","threadId":"12139","inReplyTo":"20080217181523.GA4818@coredump.intra.peff.net","subject":"Re: [RFC] sending errors to stdout under $PAGER","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-17T18:23:25Z","receivedAt":"2008-02-17T18:23:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sun, Feb 17, 2008 at 09:56:27AM -0800, Junio C Hamano wrote:\n>\n>> > Wouldn't it be better/safer to redirect stderr to the pager\n>> > in the first place?\n>> >\n>> > So, instead of the current\n>> >\n>> > \tfoo | less\n>> > use\n>> > \tfoo 2>&1 | less\n>> \n>> I like it.  Much nicer.  Thanks.\n>\n> This will also put the stderr of any sub-programs into the pager, which\n> is probably worse if you have, e.g., a chatty external diff program. I\n> don't know if we care enough about that.\n\nWe'll soon find out and the change would be a single liner that\nis very easy to back out, so let's put it in and see what\nhappens ;-).\n"},{"id":"69030","messageId":"alpine.LSU.1.00.0802171937070.30505@racer.site","threadId":"12139","inReplyTo":"7vr6fbzbky.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC] sending errors to stdout under $PAGER","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-17T19:38:23Z","receivedAt":"2008-02-17T19:38:23Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 17 Feb 2008, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > On Sun, Feb 17, 2008 at 09:56:27AM -0800, Junio C Hamano wrote:\n> >\n> >> > Wouldn't it be better/safer to redirect stderr to the pager in the \n> >> > first place?\n> >> >\n> >> > So, instead of the current\n> >> >\n> >> > \tfoo | less\n> >> > use\n> >> > \tfoo 2>&1 | less\n> >> \n> >> I like it.  Much nicer.  Thanks.\n> >\n> > This will also put the stderr of any sub-programs into the pager, \n> > which is probably worse if you have, e.g., a chatty external diff \n> > program. I don't know if we care enough about that.\n> \n> We'll soon find out and the change would be a single liner that is very \n> easy to back out, so let's put it in and see what happens ;-).\n\nI think this is a non-problem.  External diff programs are virtually all \nscripts written for git, because of the calling convention.  So all this \ndoes is tell the people who use external diff wrappers (and let them be \nchatty) to fix their scripts.\n\nCiao,\nDscho\n"}]}