{"thread":{"id":"24037","subject":"[PATCH/RFC] Fix for default pager","startedAt":"2010-06-07T23:58:08Z","lastAt":"2010-06-16T06:28:15Z","messageCount":29,"participants":["Dario Rodriguez","Ben Walton","Jeff King","Johannes Sixt","Erik Faye-Lund","Andreas Ericsson","Tor Arntsen","Miles Bader","Ævar Arnfjörð Bjarmason","Junio C Hamano","Brandon Casey","Nazri Ramliy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"143196","messageId":"1275955088-32750-1-git-send-email-soft.d4rio@gmail.com","threadId":"24037","inReplyTo":null,"subject":"[PATCH/RFC] Fix for default pager","fromName":"Dario Rodriguez","fromEmail":"soft.d4rio@gmail.com","sentAt":"2010-06-07T23:58:08Z","receivedAt":"2010-06-07T23:58:08Z","isPatch":true,"sender":{"key":"soft.d4rio@gmail.com","avatar":"https://gravatar.com/avatar/9bca8035ad520a6fc0eb66e5126adbe45992c8388c05d1f7952bd4793eb82c22?d=mp&s=160"},"body":"All commands using pager:\n\nDefault pager was 'less' even when some systems such AIX and other basic\nor old systems do NOT have 'less' installed. In such case, git just\ndoes not display anything in pager-enabled functionalities such as 'git log'\nor 'git show', exiting with status 0.\n\nWith this patch, git will not use DEFAULT_PAGER macro anymore, instead,\ngit will look for 'less' and 'more' in the most common paths.\nIf there is no pager, returns NULL as if it's 'cat'.\n\nObviously, the commit message needs to be rewritten :p\n\nSigned-off-by: Dario Rodriguez \n---\n pager.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----\n 1 files changed, 73 insertions(+), 7 deletions(-)\n\ndiff --git a/pager.c b/pager.c\nindex dac358f..3cfbd73 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -2,10 +2,19 @@\n #include \"run-command.h\"\n #include \"sigchain.h\"\n \n-#ifndef DEFAULT_PAGER\n-#define DEFAULT_PAGER \"less\"\n+#ifndef GIT_PAGER_ENVIRONMENT\n+#define GIT_PAGER_ENVIRONMENT \"GIT_PAGER\"\n #endif\n \n+#ifndef GIT_PGR_LOOKUP_DBG_ENVIRONMENT\n+#define GIT_PGR_LOOKUP_DBG_ENVIRONMENT \"GIT_PAGER_LOOKUP_DEBUG\"\n+#endif\n+\n+#ifndef PAGER_ENVIRONMENT\n+#define PAGER_ENVIRONMENT \"PAGER\"\n+#endif\n+\n+\n /*\n * This is split up from the rest of git so that we can do\n * something different on Windows.\n@@ -48,23 +57,80 @@ static void wait_for_pager_signal(int signo)\n \traise(signo);\n }\n \n-const char *git_pager(int stdout_is_tty)\n+static int is_executable(const char *name)\n+{\n+\tstruct stat st;\n+\n+\tif (stat(name, &st) ||\n+\t !S_ISREG(st.st_mode))\n+\t\treturn 0;\n+\n+#ifdef WIN32\n+{\t/* cannot trust the executable bit, peek into the file instead */\n+\tchar buf[3] = { 0 };\n+\tint n;\n+\tint fd = open(name, O_RDONLY);\n+\tst.st_mode &= ~S_IXUSR;\n+\tif (fd >= 0) {\n+\t\tn = read(fd, buf, 2);\n+\t\tif (n == 2)\n+\t\t\t/* DOS executables start with \"MZ\" */\n+\t\t\tif (!strcmp(buf, \"#!\") || !strcmp(buf, \"MZ\"))\n+\t\t\t\tst.st_mode |= S_IXUSR;\n+\t\tclose(fd);\n+\t}\n+}\n+#endif\n+\treturn st.st_mode & S_IXUSR;\n+}\n+\n+const char *git_pager(int stdout_is_tty) \n {\n+\tstatic const char *pager_bins[] =\n+\t\t{ \"less\", \"more\", NULL };\n+\tstatic const char *common_binary_paths[] =\n+\t\t{ \"/bin/\",\"/usr/bin/\",\"/usr/local/bin/\",NULL };\n+\n \tconst char *pager;\n+\tchar **p1,**p2,*pager_heap;\n+\tchar exe_name[255];\n+\tint pager_lkp_dbg=0;\n \n \tif (!stdout_is_tty)\n \t\treturn NULL;\n \n-\tpager = getenv(\"GIT_PAGER\");\n+\tif(getenv(GIT_PGR_LOOKUP_DBG_ENVIRONMENT))\n+\t\tpager_lkp_dbg=1;\n+\n+\tmemset( exe_name,0,255 );\n+\tpager = getenv(GIT_PAGER_ENVIRONMENT);\n \tif (!pager) {\n \t\tif (!pager_program)\n \t\t\tgit_config(git_default_config, NULL);\n \t\tpager = pager_program;\n \t}\n \tif (!pager)\n-\t\tpager = getenv(\"PAGER\");\n-\tif (!pager)\n-\t\tpager = DEFAULT_PAGER;\n+\t\tpager = getenv(PAGER_ENVIRONMENT);\n+\tif (!pager) {\n+\n+\t\tfor (p1=(char**)pager_bins; (*p1)&&(!pager)\n+\t\t\t;p1++)\n+\t\t\tfor (p2=(char**)common_binary_paths; (*p2)&&(!pager)\n+\t\t\t\t;p2++) {\n+\t\t\t\tsprintf( exe_name,\"%s%s\",\n+\t\t\t\t\t *p2,*p1 );\n+\t\t\t\tif (is_executable(exe_name)) {\n+\t\t\t\t\tpager_heap = (char*) malloc(sizeof(exe_name));\n+\t\t\t\t\tstrcpy(pager_heap,exe_name);\n+\t\t\t\t\tpager = pager_heap;\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\tif (pager_lkp_dbg)\n+\t\t\tfprintf(stderr, \"Debug: Lookup for existent pagers, got [%s]\\n\",\n+\t\t\t\t(pager==NULL)?\"(null)\":pager);\n+\n+\t}\n \telse if (!*pager || !strcmp(pager, \"cat\"))\n \t\tpager = NULL;\n \n-- \n1.7.1.245.g7c42e.dirty\n"},{"id":"143199","messageId":"1275955270-sup-2380@pinkfloyd.chass.utoronto.ca","threadId":"24037","inReplyTo":"1275955088-32750-1-git-send-email-soft.d4rio@gmail.com","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2010-06-08T00:04:06Z","receivedAt":"2010-06-08T00:04:06Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Dario Rodriguez's message of Mon Jun 07 19:58:08 -0400 2010:\n\n> Default pager was 'less' even when some systems such AIX and other\n> basic or old systems do NOT have 'less' installed. In such case, git\n> just does not display anything in pager-enabled functionalities such\n> as 'git log' or 'git show', exiting with status 0.\n\nWhy not set a sensible DEFAULT_PAGER value for the system in your\nconfig.mak file instead?\n\nJust curious.\n\nThanks\n-Ben\n-- \nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"143200","messageId":"AANLkTinydWk3GqGDww8FS7pmW16jAVazRkmT_GsRMIhy@mail.gmail.com","threadId":"24037","inReplyTo":"1275955270-sup-2380@pinkfloyd.chass.utoronto.ca","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Dario Rodriguez","fromEmail":"soft.d4rio@gmail.com","sentAt":"2010-06-08T02:14:58Z","receivedAt":"2010-06-08T02:14:58Z","isPatch":true,"sender":{"key":"soft.d4rio@gmail.com","avatar":"https://gravatar.com/avatar/9bca8035ad520a6fc0eb66e5126adbe45992c8388c05d1f7952bd4793eb82c22?d=mp&s=160"},"body":"On Mon, Jun 7, 2010 at 9:04 PM, Ben Walton wrote:\n>\n> Why not set a sensible DEFAULT_PAGER value for the system in your\n> config.mak file instead?\n>\n> Just curious.\n>\n\nI was thinking about it before coding the patch, and found some\nconsequences. First of all, the most important thing I should\nunderstand is that most users will install git from binary,\nprecompiled packages instead of the good download and compile. So this\nis actually a good reason to not do it that way (config.mak)... Some\nusers may download the compiled binary while it's actually calling\nit's default pager.\n\nBut this is not the only reason. Let me give you an example: We\ndevelop (I'm actually working at Accenture) using several machines.\nWhen we need some tool, we compile it in our first machine, and\ninstall it for an specific user. In some other environments we cannot\ncompile things (testing environments) so we transfer (FTP) those\nbinary files. It's just another case, and the default pager could\ncause problems here (in fact, i experienced such problems).\n\nAnother case (or something as the first one): If you installed less\n(via your package system), then git, and then you happen to uninstall\nless... should GIT be uninstalled in absence of it's default pager?\n\nI think GIT must auto-detect it's pager based in what is on the system\nat runtime, don't you think so?\n"},{"id":"143204","messageId":"20100608052929.GA15156@coredump.intra.peff.net","threadId":"24037","inReplyTo":"1275955088-32750-1-git-send-email-soft.d4rio@gmail.com","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-06-08T05:29:30Z","receivedAt":"2010-06-08T05:29:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 07, 2010 at 08:58:08PM -0300, Dario Rodriguez wrote:\n\n> Default pager was 'less' even when some systems such AIX and other basic\n> or old systems do NOT have 'less' installed. In such case, git just\n> does not display anything in pager-enabled functionalities such as 'git log'\n> or 'git show', exiting with status 0.\n> \n> With this patch, git will not use DEFAULT_PAGER macro anymore, instead,\n> git will look for 'less' and 'more' in the most common paths.\n> If there is no pager, returns NULL as if it's 'cat'.\n\nRun-time pager detection seems like a reasonable goal, I guess, but...\n\n> -const char *git_pager(int stdout_is_tty)\n> +static int is_executable(const char *name)\n> +{\n> +\tstruct stat st;\n> +\n> +\tif (stat(name, &st) ||\n> +\t !S_ISREG(st.st_mode))\n> +\t\treturn 0;\n> +\n> +#ifdef WIN32\n> +{\t/* cannot trust the executable bit, peek into the file instead */\n> +\tchar buf[3] = { 0 };\n> +\tint n;\n> +\tint fd = open(name, O_RDONLY);\n> +\tst.st_mode &= ~S_IXUSR;\n> +\tif (fd >= 0) {\n> +\t\tn = read(fd, buf, 2);\n> +\t\tif (n == 2)\n> +\t\t\t/* DOS executables start with \"MZ\" */\n> +\t\t\tif (!strcmp(buf, \"#!\") || !strcmp(buf, \"MZ\"))\n> +\t\t\t\tst.st_mode |= S_IXUSR;\n> +\t\tclose(fd);\n> +\t}\n> +}\n> +#endif\n> +\treturn st.st_mode & S_IXUSR;\n> +}\n> +\n> +const char *git_pager(int stdout_is_tty) \n> {\n> +\tstatic const char *pager_bins[] =\n> +\t\t{ \"less\", \"more\", NULL };\n> +\tstatic const char *common_binary_paths[] =\n> +\t\t{ \"/bin/\",\"/usr/bin/\",\"/usr/local/bin/\",NULL };\n\n...must we really add code with such ugliness as magic PATHs and DOS\nmagic numbers?\n\nRight now we fall back to just exec-ing \"less\". Could we instead just\ntry to exec \"less\", if that fails then \"more\", and then finally \"cat\"?\n\nThat would have almost the same effect and would be much simpler,\nwouldn't it? The exceptions I can think of are:\n\n - we would actually run \"cat\" in the final case, instead of optimizing\n it out.\n\n - \"git var GIT_PAGER\" wouldn't handle this automatically\n\n-Peff\n"},{"id":"143205","messageId":"20100608053507.GB15156@coredump.intra.peff.net","threadId":"24037","inReplyTo":"AANLkTinydWk3GqGDww8FS7pmW16jAVazRkmT_GsRMIhy@mail.gmail.com","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-06-08T05:35:07Z","receivedAt":"2010-06-08T05:35:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 07, 2010 at 11:14:58PM -0300, Dario Rodriguez wrote:\n\n> On Mon, Jun 7, 2010 at 9:04 PM, Ben Walton wrote:\n> >\n> > Why not set a sensible DEFAULT_PAGER value for the system in your\n> > config.mak file instead?\n> >\n> > Just curious.\n> >\n> \n> I was thinking about it before coding the patch, and found some\n> consequences. First of all, the most important thing I should\n> understand is that most users will install git from binary,\n> precompiled packages instead of the good download and compile. So this\n> is actually a good reason to not do it that way (config.mak)... Some\n> users may download the compiled binary while it's actually calling\n> it's default pager.\n\nIf you are downloading a binary, the package compiler should do one of\ntwo things:\n\n 1. indicate a package dependency on 'less'\n\n 2. set DEFAULT_PAGER to 'more' (or whatever is appropriate for your\n system)\n\nYes, auto-detection means we can more flexibly \"upgrade\" to less when\nthe package suddenly appears. But if you really care about your pager,\nwhy not just set $PAGER?\n\nThe most important thing is that users who _don't_ care don't see\nsomething broken, but the rules above already cover that with current\ngit.\n\n> But this is not the only reason. Let me give you an example: We\n> develop (I'm actually working at Accenture) using several machines.\n> When we need some tool, we compile it in our first machine, and\n> install it for an specific user. In some other environments we cannot\n> compile things (testing environments) so we transfer (FTP) those\n> binary files. It's just another case, and the default pager could\n> cause problems here (in fact, i experienced such problems).\n\nThen set DEFAULT_PAGER to 'more' (or 'cat' for that matter), and use\n$PAGER on machines that are more capable.\n\n-Peff\n"},{"id":"143207","messageId":"4C0DDF53.5090805@viscovery.net","threadId":"24037","inReplyTo":"20100608052929.GA15156@coredump.intra.peff.net","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-06-08T06:12:35Z","receivedAt":"2010-06-08T06:12:35Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 6/8/2010 7:29, schrieb Jeff King:\n> On Mon, Jun 07, 2010 at 08:58:08PM -0300, Dario Rodriguez wrote:\n>> +#ifdef WIN32\n>> +{\t/* cannot trust the executable bit, peek into the file instead */\n>> +\tchar buf[3] = { 0 };\n>> +\tint n;\n>> +\tint fd = open(name, O_RDONLY);\n>> +\tst.st_mode &= ~S_IXUSR;\n>> +\tif (fd >= 0) {\n>> +\t\tn = read(fd, buf, 2);\n>> +\t\tif (n == 2)\n>> +\t\t\t/* DOS executables start with \"MZ\" */\n>> +\t\t\tif (!strcmp(buf, \"#!\") || !strcmp(buf, \"MZ\"))\n>> +\t\t\t\tst.st_mode |= S_IXUSR;\n>> +\t\tclose(fd);\n>> +\t}\n>> +}\n>> +#endif\n>> +\treturn st.st_mode & S_IXUSR;\n>> +}\n>> +\n>> +const char *git_pager(int stdout_is_tty) \n>> {\n>> +\tstatic const char *pager_bins[] =\n>> +\t\t{ \"less\", \"more\", NULL };\n>> +\tstatic const char *common_binary_paths[] =\n>> +\t\t{ \"/bin/\",\"/usr/bin/\",\"/usr/local/bin/\",NULL };\n> \n> ...must we really add code with such ugliness as magic PATHs and DOS\n> magic numbers?\n\nYes and no.\n\nCoding DOS magic numbers is bad because you could wrapped a real pager in\na shell script (even on Windows).\n\nThese days, start_command() is able to report ENOENT if it cannot run a\nprogram because it does not exist. However, right in the case of\ngit_pager() this capability is disabled because it sets this magic\npreexec_cb that waits for data in the child process before it execs the\nreal pager.\n\nIf we could get rid of preexec_cb, we could make this work much more\npleasantly. pager_preexec waits for data on stdin; Linus added it to work\naround a faulty 'less'. Do we still need it?\n\n-- Hannes\n"},{"id":"143227","messageId":"AANLkTinxcrIV2TM966EkOC_crR0bHdNllEIdibz4gGjd@mail.gmail.com","threadId":"24037","inReplyTo":"20100608052929.GA15156@coredump.intra.peff.net","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Dario Rodriguez","fromEmail":"soft.d4rio@gmail.com","sentAt":"2010-06-08T12:24:45Z","receivedAt":"2010-06-08T12:24:45Z","isPatch":true,"sender":{"key":"soft.d4rio@gmail.com","avatar":"https://gravatar.com/avatar/9bca8035ad520a6fc0eb66e5126adbe45992c8388c05d1f7952bd4793eb82c22?d=mp&s=160"},"body":"On Tue, Jun 8, 2010 at 2:29 AM, Jeff King wrote:\n> On Mon, Jun 07, 2010 at 08:58:08PM -0300, Dario Rodriguez wrote:\n>\n>> Default pager was 'less' even when some systems such AIX and other basic\n>> or old systems do NOT have 'less' installed. In such case, git just\n>> does not display anything in pager-enabled functionalities such as 'git log'\n>> or 'git show', exiting with status 0.\n>>\n>> With this patch, git will not use DEFAULT_PAGER macro anymore, instead,\n>> git will look for 'less' and 'more' in the most common paths.\n>> If there is no pager, returns NULL as if it's 'cat'.\n>\n> Run-time pager detection seems like a reasonable goal, I guess, but...\n>\n>> -const char *git_pager(int stdout_is_tty)\n>> +static int is_executable(const char *name)\n>> +{\n>> +     struct stat st;\n>> +\n>> +     if (stat(name, &st) ||\n>> +         !S_ISREG(st.st_mode))\n>> +             return 0;\n>> +\n>> +#ifdef WIN32\n>> +{    /* cannot trust the executable bit, peek into the file instead */\n>> +     char buf[3] = { 0 };\n>> +     int n;\n>> +     int fd = open(name, O_RDONLY);\n>> +     st.st_mode &= ~S_IXUSR;\n>> +     if (fd >= 0) {\n>> +             n = read(fd, buf, 2);\n>> +             if (n == 2)\n>> +                     /* DOS executables start with \"MZ\" */\n>> +                     if (!strcmp(buf, \"#!\") || !strcmp(buf, \"MZ\"))\n>> +                             st.st_mode |= S_IXUSR;\n>> +             close(fd);\n>> +     }\n>> +}\n>> +#endif\n>> +     return st.st_mode & S_IXUSR;\n>> +}\n>> +\n>> +const char *git_pager(int stdout_is_tty)\n>>  {\n>> +     static const char *pager_bins[] =\n>> +             { \"less\", \"more\", NULL };\n>> +     static const char *common_binary_paths[] =\n>> +             { \"/bin/\",\"/usr/bin/\",\"/usr/local/bin/\",NULL };\n>\n> ...must we really add code with such ugliness as magic PATHs and DOS\n> magic numbers?\n>\n\nI copied the function 'is_executable' from 'help.c' so we already have\nsuch code... :p\n\n> Right now we fall back to just exec-ing \"less\". Could we instead just\n> try to exec \"less\", if that fails then \"more\", and then finally \"cat\"?\n>\n\nis such a good idea but right now, 'git_pager' is not exec-ing, it's\njust setting up a pager. If you set-up the pager based on wich one\nfails in it's execution, you must avoid usage of this function, since\nit will always return 'less' (or 'more...). What I posted is\ntransparent to any other function; 'git_pager' will be called\nreturning an existent, working pager, so the flow is the same, however\nI like your proposal too, and should be considered.\n\n> That would have almost the same effect and would be much simpler,\n> wouldn't it? The exceptions I can think of are:\n>\n>  - we would actually run \"cat\" in the final case, instead of optimizing\n>    it out.\n>\n\nActually pager is being set to NULL if it's 'cat'... what's git doing\nwith a NULL pager?\n\n>  - \"git var GIT_PAGER\" wouldn't handle this automatically\n>\n> -Peff\n>\n\nBlessing,\nDario\n"},{"id":"143234","messageId":"AANLkTilvvpy4TBQF6g8boQL87FRB7kFDrVfYiHvOv6xu@mail.gmail.com","threadId":"24037","inReplyTo":"20100608053507.GB15156@coredump.intra.peff.net","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Dario Rodriguez","fromEmail":"soft.d4rio@gmail.com","sentAt":"2010-06-08T13:49:51Z","receivedAt":"2010-06-08T13:49:51Z","isPatch":true,"sender":{"key":"soft.d4rio@gmail.com","avatar":"https://gravatar.com/avatar/9bca8035ad520a6fc0eb66e5126adbe45992c8388c05d1f7952bd4793eb82c22?d=mp&s=160"},"body":"On Tue, Jun 8, 2010 at 2:35 AM, Jeff King wrote:\n> If you are downloading a binary, the package compiler should do one of\n> two things:\n>\n>  1. indicate a package dependency on 'less'\n>\n\nbut... git will be uninstalled if you happen to uninstall less.\n\n>  2. set DEFAULT_PAGER to 'more' (or whatever is appropriate for your\n>     system)\n>\n\nand I know it's a very nice fallback default, but you still depending\non the pager, and will display nothing and return 0 if 'more' fails to\nexecute.\n\nSomething needs to be changed... if the sane way is to keep default\npager (almost setting 'more') instead of auto-detection... then we\nmust say something to the user when the pager fails to execute. (*)\nActually 'git' display nothing... and returns 0.\n\n> Yes, auto-detection means we can more flexibly \"upgrade\" to less when\n> the package suddenly appears. But if you really care about your pager,\n> why not just set $PAGER?\n>\n\ngood point, I agree. But again, back to (*), we must correct\nsomething... the other way is that if pager fails to execute, we\ncannot simply return 0.\n\nI could submit a patch for this, may be is a little bit better (or simpler).\n"},{"id":"143237","messageId":"AANLkTikoHwupz8ZycLKko_gaZcsAnnXjlWIU4qpt-_9T@mail.gmail.com","threadId":"24037","inReplyTo":"AANLkTinxcrIV2TM966EkOC_crR0bHdNllEIdibz4gGjd@mail.gmail.com","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2010-06-08T14:04:44Z","receivedAt":"2010-06-08T14:04:44Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Jun 8, 2010 at 2:24 PM, Dario Rodriguez wrote:\n> On Tue, Jun 8, 2010 at 2:29 AM, Jeff King wrote:\n>> On Mon, Jun 07, 2010 at 08:58:08PM -0300, Dario Rodriguez wrote:\n>>\n>>> Default pager was 'less' even when some systems such AIX and other basic\n>>> or old systems do NOT have 'less' installed. In such case, git just\n>>> does not display anything in pager-enabled functionalities such as 'git log'\n>>> or 'git show', exiting with status 0.\n>>>\n>>> With this patch, git will not use DEFAULT_PAGER macro anymore, instead,\n>>> git will look for 'less' and 'more' in the most common paths.\n>>> If there is no pager, returns NULL as if it's 'cat'.\n>>\n>> Run-time pager detection seems like a reasonable goal, I guess, but...\n>>\n>>> -const char *git_pager(int stdout_is_tty)\n>>> +static int is_executable(const char *name)\n>>> +{\n>>> +     struct stat st;\n>>> +\n>>> +     if (stat(name, &st) ||\n>>> +         !S_ISREG(st.st_mode))\n>>> +             return 0;\n>>> +\n>>> +#ifdef WIN32\n>>> +{    /* cannot trust the executable bit, peek into the file instead */\n>>> +     char buf[3] = { 0 };\n>>> +     int n;\n>>> +     int fd = open(name, O_RDONLY);\n>>> +     st.st_mode &= ~S_IXUSR;\n>>> +     if (fd >= 0) {\n>>> +             n = read(fd, buf, 2);\n>>> +             if (n == 2)\n>>> +                     /* DOS executables start with \"MZ\" */\n>>> +                     if (!strcmp(buf, \"#!\") || !strcmp(buf, \"MZ\"))\n>>> +                             st.st_mode |= S_IXUSR;\n>>> +             close(fd);\n>>> +     }\n>>> +}\n>>> +#endif\n>>> +     return st.st_mode & S_IXUSR;\n>>> +}\n>>> +\n>>> +const char *git_pager(int stdout_is_tty)\n>>>  {\n>>> +     static const char *pager_bins[] =\n>>> +             { \"less\", \"more\", NULL };\n>>> +     static const char *common_binary_paths[] =\n>>> +             { \"/bin/\",\"/usr/bin/\",\"/usr/local/bin/\",NULL };\n>>\n>> ...must we really add code with such ugliness as magic PATHs and DOS\n>> magic numbers?\n>>\n>\n> I copied the function 'is_executable' from 'help.c' so we already have\n> such code... :p\n>\n\nHow about just un-staticifying the version in help.c instead of\nduplicating it, then? That way we'd have half the ugliness for this\npurpose...\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"143239","messageId":"4C0E5103.7030501@viscovery.net","threadId":"24037","inReplyTo":"AANLkTilvvpy4TBQF6g8boQL87FRB7kFDrVfYiHvOv6xu@mail.gmail.com","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-06-08T14:17:39Z","receivedAt":"2010-06-08T14:17:39Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 6/8/2010 15:49, schrieb Dario Rodriguez:\n> we must correct\n> something... the other way is that if pager fails to execute, we\n> cannot simply return 0.\n\nBut we do not return 0:\n\n $ GIT_PAGER=/is/not/there git log\n $ echo $?\n 141\n\nThat's SIGPIPE, just as I would expect.\n\nAnd with this change\n\ndiff --git a/pager.c b/pager.c\nindex dac358f..86519cc 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -89,9 +89,6 @@ void setup_pager(void)\n \t\tstatic const char *env[] = { \"LESS=FRSX\", NULL };\n \t\tpager_process.env = env;\n \t}\n-#ifndef WIN32\n-\tpager_process.preexec_cb = pager_preexec;\n-#endif\n \tif (start_command(&pager_process))\n \treturn;\n\n\nI get:\n\n $ GIT_PAGER=/is/not/there ./git log -1 --oneline\n error: cannot run /is/not/there: No such file or directory\n 1a16cee merge-recursive: demonstrate an incorrect conflict with submodule\n\n-- Hannes\n"},{"id":"143240","messageId":"AANLkTilWg8hw5j20o-xGsVO-q_OeSmtKEKAO6O416qvH@mail.gmail.com","threadId":"24037","inReplyTo":"4C0E5103.7030501@viscovery.net","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Dario Rodriguez","fromEmail":"soft.d4rio@gmail.com","sentAt":"2010-06-08T14:39:38Z","receivedAt":"2010-06-08T14:39:38Z","isPatch":true,"sender":{"key":"soft.d4rio@gmail.com","avatar":"https://gravatar.com/avatar/9bca8035ad520a6fc0eb66e5126adbe45992c8388c05d1f7952bd4793eb82c22?d=mp&s=160"},"body":"On Tue, Jun 8, 2010 at 11:17 AM, Johannes Sixt wrote:\n> Am 6/8/2010 15:49, schrieb Dario Rodriguez:\n>> we must correct\n>> something... the other way is that if pager fails to execute, we\n>> cannot simply return 0.\n>\n> But we do not return 0:\n>\n>  $ GIT_PAGER=/is/not/there git log\n>  $ echo $?\n>  141\n>\n> That's SIGPIPE, just as I would expect.\n>\n\nAs I said in the original thread...\n\n$ PAGER=/nothing/here ../git log\n$ echo $?\n0\n\n$ GIT_PAGER=/nothing/here ../git log\n$ echo $?\n0\n\nThat's on AIX 5.2\n\n\n> And with this change\n>\n> diff --git a/pager.c b/pager.c\n> index dac358f..86519cc 100644\n> --- a/pager.c\n> +++ b/pager.c\n> @@ -89,9 +89,6 @@ void setup_pager(void)\n>                static const char *env[] = { \"LESS=FRSX\", NULL };\n>                pager_process.env = env;\n>        }\n> -#ifndef WIN32\n> -       pager_process.preexec_cb = pager_preexec;\n> -#endif\n>        if (start_command(&pager_process))\n>        return;\n>\n>\n> I get:\n>\n>  $ GIT_PAGER=/is/not/there ./git log -1 --oneline\n>  error: cannot run /is/not/there: No such file or directory\n>  1a16cee merge-recursive: demonstrate an incorrect conflict with submodule\n>\n\nCurious... I patched it on AIX and I get:\n\n$ GIT_PAGER=/nothing/here ../git log\nerror: cannot run /nothing/here: No such file or directory\ncommit 3274a12f940680612e3bfd3d022a0eab460c0f1f\nAuthor: usuario ####### <#######@Maquina01.(none)>\nDate: Thu Jun 3 20:02:23 2010 +0200\n\n OtherCom\n\ncommit acf110f7c878a37e4a5af8499134df28da0e8ab3\nAuthor: usuario ####### <#######@Maquina01.(none)>\nDate: Thu Jun 3 20:01:37 2010 +0200\n\n inicial\n\n\n\nHowever, the patch must delete the pager_preexec definition too... but\nI wonder, do somebody still need it?\n\nbtw: I still think 'more' is much more sane fallback default than\n'less'... look (with your patch applied):\n\n$ ../git log\nerror: cannot run less: No such file or directory\ncommit 3274a12f940680612e3bfd3d022a0eab460c0f1f\nAuthor: ####### <#######@Maquina01.(none)>\nDate: Thu Jun 3 20:02:23 2010 +0200\n\n OtherCom\n\ncommit acf110f7c878a37e4a5af8499134df28da0e8ab3\nAuthor: ####### <#######@Maquina01.(none)>\nDate: Thu Jun 3 20:01:37 2010 +0200\n\n inicial\n"},{"id":"143248","messageId":"4C0E6810.3070301@viscovery.net","threadId":"24037","inReplyTo":"AANLkTilWg8hw5j20o-xGsVO-q_OeSmtKEKAO6O416qvH@mail.gmail.com","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-06-08T15:56:00Z","receivedAt":"2010-06-08T15:56:00Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 08.06.2010 16:39, schrieb Dario Rodriguez:\n> On Tue, Jun 8, 2010 at 11:17 AM, Johannes Sixt wrote:\n>> $ GIT_PAGER=/is/not/there git log\n>> $ echo $?\n>> 141\n>>\n>> That's SIGPIPE, just as I would expect.\n>\n> As I said in the original thread...\n>\n> $ PAGER=/nothing/here ../git log\n> $ echo $?\n> 0\n\nThat's no surprise with your toy repository: git-log has run to completion \n(without overrunning the pipe buffer) before the pager process that it \nforked can even execute its first instruction.\n\n> btw: I still think 'more' is much more sane fallback default than\n> 'less'... look (with your patch applied):\n>\n> $ ../git log\n> error: cannot run less: No such file or directory\n> commit 3274a12f940680612e3bfd3d022a0eab460c0f1f\n> Author: #######<#######@Maquina01.(none)>\n> Date: Thu Jun 3 20:02:23 2010 +0200\n>\n> OtherCom\n>\n> commit acf110f7c878a37e4a5af8499134df28da0e8ab3\n> Author: #######<#######@Maquina01.(none)>\n> Date: Thu Jun 3 20:01:37 2010 +0200\n>\n> inicial\n>\n\nHow is this an argument for 'more'? (Just asking; I don't see your point.)\n\n-- Hannes\n"},{"id":"143256","messageId":"AANLkTinZSuXJEXzpvEavYNLSyqUlx8qzWlrbtIH6q6fx@mail.gmail.com","threadId":"24037","inReplyTo":"4C0E6810.3070301@viscovery.net","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Dario Rodriguez","fromEmail":"soft.d4rio@gmail.com","sentAt":"2010-06-08T17:28:16Z","receivedAt":"2010-06-08T17:28:16Z","isPatch":true,"sender":{"key":"soft.d4rio@gmail.com","avatar":"https://gravatar.com/avatar/9bca8035ad520a6fc0eb66e5126adbe45992c8388c05d1f7952bd4793eb82c22?d=mp&s=160"},"body":"On Tue, Jun 8, 2010 at 12:56 PM, Johannes Sixt wrote:\n> Am 08.06.2010 16:39, schrieb Dario Rodriguez:\n>>\n>> On Tue, Jun 8, 2010 at 11:17 AM, Johannes Sixt\n>>  wrote:\n>>>\n>>>  $ GIT_PAGER=/is/not/there git log\n>>>  $ echo $?\n>>>  141\n>>>\n>>> That's SIGPIPE, just as I would expect.\n>>\n>> As I said in the original thread...\n>>\n>> $ PAGER=/nothing/here ../git log\n>> $ echo $?\n>> 0\n>\n> That's no surprise with your toy repository: git-log has run to completion\n> (without overrunning the pipe buffer) before the pager process that it\n> forked can even execute its first instruction.\n>\n\nI cannot understand what's the point... I'm running git without\ninstalling it, but why do you say \"toy repository\"?...\n\n>> btw: I still think 'more' is much more sane fallback default than\n>> 'less'... look (with your patch applied):\n>>\n>> $ ../git log\n>> error: cannot run less: No such file or directory\n>> commit 3274a12f940680612e3bfd3d022a0eab460c0f1f\n>> Author: #######<#######@Maquina01.(none)>\n>> Date:   Thu Jun 3 20:02:23 2010 +0200\n>>\n>>     OtherCom\n>>\n>> commit acf110f7c878a37e4a5af8499134df28da0e8ab3\n>> Author: #######<#######@Maquina01.(none)>\n>> Date:   Thu Jun 3 20:01:37 2010 +0200\n>>\n>>     inicial\n>>\n>\n> How is this an argument for 'more'? (Just asking; I don't see your point.)\n>\n\nNo problem, let me explain: 'more' is older and standard while\n(correct me if not true) 'less' is not in POSIX:2008. I work in a lot\nof Unix-like systems and I found 'more' as a standard, while less is\njust sometimes installed... my error running 'less' is such an\nargument for 'more' because:\n\n$ PAGER=more ../git log\ncommit fccd640197e34dbff72954924ed5c76c42a4aac7\nAuthor: ###### <######@Maquina01.(none)>\nDate: Tue Jun 8 19:23:23 2010 +0200\n\n commit\nstdin: END\n\n'more' is not a problem... and as a POSIX standard it's almost never a problem.\n"},{"id":"143264","messageId":"4C0E932B.3010702@viscovery.net","threadId":"24037","inReplyTo":"AANLkTinZSuXJEXzpvEavYNLSyqUlx8qzWlrbtIH6q6fx@mail.gmail.com","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-06-08T18:59:55Z","receivedAt":"2010-06-08T18:59:55Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 08.06.2010 19:28, schrieb Dario Rodriguez:\n> On Tue, Jun 8, 2010 at 12:56 PM, Johannes Sixt wrote:\n>> Am 08.06.2010 16:39, schrieb Dario Rodriguez:\n>>>\n>>> On Tue, Jun 8, 2010 at 11:17 AM, Johannes Sixt\n>>> wrote:\n>>>>\n>>>> $ GIT_PAGER=/is/not/there git log\n>>>> $ echo $?\n>>>> 141\n>>>>\n>>>> That's SIGPIPE, just as I would expect.\n>>>\n>>> As I said in the original thread...\n>>>\n>>> $ PAGER=/nothing/here ../git log\n>>> $ echo $?\n>>> 0\n>>\n>> That's no surprise with your toy repository: git-log has run to completion\n>> (without overrunning the pipe buffer) before the pager process that it\n>> forked can even execute its first instruction.\n>>\n>\n> I cannot understand what's the point... I'm running git without\n> installing it, but why do you say \"toy repository\"?...\n\nYour repository has only 2 commits and its git log output is less than \n1kB, i.e., sufficiently small to fit in a pipe's buffer.\n\ngit log calls start_command to fork() the pager. The OS's scheduler does \nnot run the newly forked process immediately; rather, git log goes on with \nits own business, writing output to the pipe that connects to the pager. \nBecause your repository is so small, git log never has to wait that the \npager drains the pipe. git log finally reaches exit(0). At this time, an \natexit() handler (wait_for_pager()) finally calls finish_command() to wait \nfor the pager.\n\nThis is the first time that the forked child process can run. Only now it \nturns out that the pager cannot be run. The child process closes the pipe \nand exits with an error, but it is too late: wait_for_pager() drops the \nerror return code of finish_command() to the floor. The parent process \n(git log) can complete with the exit code that it was given earlier, 0.\n\nRepeat your experiment with ./git log in git.git itself to see the difference.\n\n-- Hannes\n"},{"id":"143274","messageId":"AANLkTinB_SBilMOfgnHtDrQS-NBOLF4yY5NaP7ZvN9rK@mail.gmail.com","threadId":"24037","inReplyTo":"4C0E932B.3010702@viscovery.net","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Dario Rodriguez","fromEmail":"soft.d4rio@gmail.com","sentAt":"2010-06-08T20:44:25Z","receivedAt":"2010-06-08T20:44:25Z","isPatch":true,"sender":{"key":"soft.d4rio@gmail.com","avatar":"https://gravatar.com/avatar/9bca8035ad520a6fc0eb66e5126adbe45992c8388c05d1f7952bd4793eb82c22?d=mp&s=160"},"body":"On Tue, Jun 8, 2010 at 3:59 PM, Johannes Sixt wrote:\n> Your repository has only 2 commits and its git log output is less than 1kB,\n> i.e., sufficiently small to fit in a pipe's buffer.\n>\n> git log calls start_command to fork() the pager. The OS's scheduler does not\n> run the newly forked process immediately; rather, git log goes on with its\n> own business, writing output to the pipe that connects to the pager. Because\n> your repository is so small, git log never has to wait that the pager drains\n> the pipe. git log finally reaches exit(0). At this time, an atexit() handler\n> (wait_for_pager()) finally calls finish_command() to wait for the pager.\n>\n> This is the first time that the forked child process can run. Only now it\n> turns out that the pager cannot be run. The child process closes the pipe\n> and exits with an error, but it is too late: wait_for_pager() drops the\n> error return code of finish_command() to the floor. The parent process (git\n> log) can complete with the exit code that it was given earlier, 0.\n>\n> Repeat your experiment with ./git log in git.git itself to see the\n> difference.\n>\n> -- Hannes\n>\n\nCapisco & touché, with much more than 1k of info, git show ends with a\n\"Broken Pipe\"... seems hard to detect for little, recently started\nprojects since I added more than 60k of scripts and I need to do 'git\nshow' to understand that the problem is a broken pipe.\n\nNow, let me think about it... do we need the pager_preexec function? I\nmean... it works fine without it, and the function is there because of\na faulty 'less'.\n\nMy problem is obvioulsly solved by adding PAGER=more in my default\nenvironment, but I think this could be a litle bit embarrassing for a\nnew user, mostly in environments such this AIX :P\n\nCheers,\nDario\n"},{"id":"143283","messageId":"4C0EB741.9020905@op5.se","threadId":"24037","inReplyTo":"AANLkTinB_SBilMOfgnHtDrQS-NBOLF4yY5NaP7ZvN9rK@mail.gmail.com","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2010-06-08T21:33:53Z","receivedAt":"2010-06-08T21:33:53Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"On 06/08/2010 10:44 PM, Dario Rodriguez wrote:\n> On Tue, Jun 8, 2010 at 3:59 PM, Johannes Sixt wrote:\n>> Your repository has only 2 commits and its git log output is less than 1kB,\n>> i.e., sufficiently small to fit in a pipe's buffer.\n>>\n>> git log calls start_command to fork() the pager. The OS's scheduler does not\n>> run the newly forked process immediately; rather, git log goes on with its\n>> own business, writing output to the pipe that connects to the pager. Because\n>> your repository is so small, git log never has to wait that the pager drains\n>> the pipe. git log finally reaches exit(0). At this time, an atexit() handler\n>> (wait_for_pager()) finally calls finish_command() to wait for the pager.\n>>\n>> This is the first time that the forked child process can run. Only now it\n>> turns out that the pager cannot be run. The child process closes the pipe\n>> and exits with an error, but it is too late: wait_for_pager() drops the\n>> error return code of finish_command() to the floor. The parent process (git\n>> log) can complete with the exit code that it was given earlier, 0.\n>>\n>> Repeat your experiment with ./git log in git.git itself to see the\n>> difference.\n>>\n>> -- Hannes\n>>\n> \n> Capisco& touché, with much more than 1k of info, git show ends with a\n> \"Broken Pipe\"... seems hard to detect for little, recently started\n> projects since I added more than 60k of scripts and I need to do 'git\n> show' to understand that the problem is a broken pipe.\n> \n> Now, let me think about it... do we need the pager_preexec function? I\n> mean... it works fine without it, and the function is there because of\n> a faulty 'less'.\n> \n> My problem is obvioulsly solved by adding PAGER=more in my default\n> environment, but I think this could be a litle bit embarrassing for a\n> new user, mostly in environments such this AIX :P\n> \n\nCatering to AIX by default seems stupid beyond belief. AIX users today\nare, without fail, accustomed to having to tweak more or less everything\nto make the system run smoothly with modern applications (where \"modern\"\nis a generous term, including everything that's been written in the last\n10 or so years).\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n\nConsidering the successes of the wars on alcohol, poverty, drugs and\nterror, I think we should give some serious thought to declaring war\non peace.\n"},{"id":"143317","messageId":"AANLkTinAO5empFix9W_rbtU3Vv4O73OsJBtA1stb66DS@mail.gmail.com","threadId":"24037","inReplyTo":"4C0EB741.9020905@op5.se","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Tor Arntsen","fromEmail":"tor@spacetec.no","sentAt":"2010-06-09T09:08:49Z","receivedAt":"2010-06-09T09:08:49Z","isPatch":true,"sender":{"key":"tor@spacetec.no","avatar":null},"body":"On Tue, Jun 8, 2010 at 23:33, Andreas Ericsson wrote:\n\n> Catering to AIX by default seems stupid beyond belief. AIX users today\n> are, without fail, accustomed to having to tweak more or less everything\n> to make the system run smoothly with modern applications (where \"modern\"\n> is a generous term, including everything that's been written in the last\n> 10 or so years).\n\nAIX doesn't come with 'less', because it doesn't really need one.\n'more' on AIX can page backwards/forwards in piped data (unlike 'more'\non Linux etc.), thus negating the most common need for installing\n'less' elsewhere. (The other big feature of less, that it by default\nclears the screen just as you had found what you wanted to read and\nwere about to apply it to the command line before 'poof' - gone) is so\nannoying to me that I routinely sets PAGER=more before using Git.\n\nIf it's true that Git should demand 'less' to be installed before\nbeing usable out of the box.. well, that's just plain silly.\n\n-Tor\n"},{"id":"143319","messageId":"buo4ohcsgdu.fsf@dhlpc061.dev.necel.com","threadId":"24037","inReplyTo":"AANLkTinAO5empFix9W_rbtU3Vv4O73OsJBtA1stb66DS@mail.gmail.com","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2010-06-09T09:29:49Z","receivedAt":"2010-06-09T09:29:49Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Tor Arntsen writes:\n> AIX doesn't come with 'less', because it doesn't really need one.\n> 'more' on AIX can page backwards/forwards in piped data (unlike 'more'\n> on Linux etc.), thus negating the most common need for installing\n> 'less' elsewhere.\n\nLess has about a zillion features that more [traditionally] doesn't\nhave, and even some of the more esoteric ones can be quite useful for\ngit. For instance you can determine in a great deal of detail exactly\nhow/when/if it clears the screen/shows a prompt/waits for user input\nbefore exiting, etc.\n\n-Miles\n\n-- \n(\\(\\\n(^.^)\n(\")\")\n*This is the cute bunny virus, please copy this into your sig so it can spread.\n"},{"id":"143367","messageId":"AANLkTikqnMGviOOrTWDUA82u3pwfWnunzj8KxQkRcLsm@mail.gmail.com","threadId":"24037","inReplyTo":"4C0EB741.9020905@op5.se","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-06-09T18:57:33Z","receivedAt":"2010-06-09T18:57:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jun 8, 2010 at 21:33, Andreas Ericsson wrote:\n> Catering to AIX by default seems stupid beyond belief. AIX users today\n> are, without fail, accustomed to having to tweak more or less everything\n> to make the system run smoothly with modern applications (where \"modern\"\n> is a generous term, including everything that's been written in the last\n> 10 or so years).\n\nBSD users are also accustomed to that to a smaller degree. But as long\nas they keep submitting portability patches to keep their ports up to\ndate, I don't see why those patches shouldn't be accepted. If they're\notherwise fit for inclusion that is.\n"},{"id":"143402","messageId":"20100610082916.GA5559@coredump.intra.peff.net","threadId":"24037","inReplyTo":"AANLkTinAO5empFix9W_rbtU3Vv4O73OsJBtA1stb66DS@mail.gmail.com","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-06-10T08:29:16Z","receivedAt":"2010-06-10T08:29:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 09, 2010 at 11:08:49AM +0200, Tor Arntsen wrote:\n\n> If it's true that Git should demand 'less' to be installed before\n> being usable out of the box.. well, that's just plain silly.\n\nIt depends on how you define \"out of the box\". The person compiling it\njust needs to set DEFAULT_PAGER appropriately for their system. \"less\"\nis a sane choice for most modern systems. But we can make it even easier\non AIX people with:\n\ndiff --git a/Makefile b/Makefile\nindex 34b7dd5..6ad0aca 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD)\n \tHAVE_PATHS_H = YesPlease\n endif\n ifeq ($(uname_S),AIX)\n+\tDEFAULT_PAGER = more\n \tNO_STRCASESTR=YesPlease\n \tNO_MEMMEM = YesPlease\n \tNO_MKDTEMP = YesPlease\n\nThat won't do automagic run-time detection if you have \"less\" installed,\nbut given your claim that AIX's \"more\" actually doesn't suck, it's\nprobably a good default. People who care can set their PAGER environment\nvariable.\n\n-Peff\n"},{"id":"143403","messageId":"AANLkTinLt3p0q-q5oDFk5CWzdhqQ2lwkWuvpdPzKZvYe@mail.gmail.com","threadId":"24037","inReplyTo":"20100610082916.GA5559@coredump.intra.peff.net","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Tor Arntsen","fromEmail":"tor@spacetec.no","sentAt":"2010-06-10T08:48:31Z","receivedAt":"2010-06-10T08:48:31Z","isPatch":true,"sender":{"key":"tor@spacetec.no","avatar":null},"body":"On Thu, Jun 10, 2010 at 10:29, Jeff King wrote:\n> On Wed, Jun 09, 2010 at 11:08:49AM +0200, Tor Arntsen wrote:\n>\n>> If it's true that Git should demand 'less' to be installed before\n>> being usable out of the box.. well, that's just plain silly.\n>\n> It depends on how you define \"out of the box\". The person compiling it\n> just needs to set DEFAULT_PAGER appropriately for their system. \"less\"\n> is a sane choice for most modern systems. But we can make it even easier\n> on AIX people with:\n>\n> diff --git a/Makefile b/Makefile\n> index 34b7dd5..6ad0aca 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD)\n>        HAVE_PATHS_H = YesPlease\n>  endif\n>  ifeq ($(uname_S),AIX)\n> +       DEFAULT_PAGER = more\n>        NO_STRCASESTR=YesPlease\n>        NO_MEMMEM = YesPlease\n>        NO_MKDTEMP = YesPlease\n\nThat looks good to me.\n\n> That won't do automagic run-time detection if you have \"less\" installed,\n> but given your claim that AIX's \"more\" actually doesn't suck, it's\n> probably a good default.\n\nIt does the paging just as 'less', yes. I can page 'git log' or any\nother output. backwards/forwards as I wish. I'm not aware of any\n'more' other than the AIX one that does this. Even the archaic\n(pre-AIX5) versions did. The AIX 'more' also have several more\nin-buffer options compared to e.g. Linux 'more'. It sits somewhere\nbetween traditional 'more' and 'less', feature-wise. (The AIX 6\nversion even does the, to me, annoying less-style erase-screen, unless\nturned off by an option or environment variable.)\n\n> People who care can set their PAGER environment\n> variable.\n\n-Tor\n"},{"id":"143404","messageId":"20100610085952.GA8269@coredump.intra.peff.net","threadId":"24037","inReplyTo":"AANLkTinLt3p0q-q5oDFk5CWzdhqQ2lwkWuvpdPzKZvYe@mail.gmail.com","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-06-10T08:59:52Z","receivedAt":"2010-06-10T08:59:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 10, 2010 at 10:48:31AM +0200, Tor Arntsen wrote:\n\n> That looks good to me.\n\nOK, here it is with a commit message. Other systems might want the same,\nI guess (Solaris, IRIX?). I'm cc'ing Brandon, who might have some input.\n\nNote that this is completely untested by me, as all of my AIX boxen have\ngone away in the past few months (yay!).\n\n-- >8 --\nSubject: [PATCH] Makefile: default pager on AIX to \"more\"\n\nAIX doesn't ship with \"less\" by default, and their \"more\" is\nmore featureful than average, so the latter is a more\nsensible choice. People who really want less can set the\ncompile-time option themselves, or users can set $PAGER.\n\nSigned-off-by: Jeff King \n---\n Makefile | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 34b7dd5..6ad0aca 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD)\n \tHAVE_PATHS_H = YesPlease\n endif\n ifeq ($(uname_S),AIX)\n+\tDEFAULT_PAGER = more\n \tNO_STRCASESTR=YesPlease\n \tNO_MEMMEM = YesPlease\n \tNO_MKDTEMP = YesPlease\n-- \n1.7.1.514.g71ed8\n"},{"id":"143408","messageId":"4C10AF43.5050708@spacetec.no","threadId":"24037","inReplyTo":"20100610085952.GA8269@coredump.intra.peff.net","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Tor Arntsen","fromEmail":"tor@spacetec.no","sentAt":"2010-06-10T09:24:19Z","receivedAt":"2010-06-10T09:24:19Z","isPatch":true,"sender":{"key":"tor@spacetec.no","avatar":null},"body":"On 06/10/10 10:59, Jeff King wrote:\n> On Thu, Jun 10, 2010 at 10:48:31AM +0200, Tor Arntsen wrote:\n> \n>> That looks good to me.\n> \n> OK, here it is with a commit message. Other systems might want the same,\n> I guess (Solaris, IRIX?). I'm cc'ing Brandon, who might have some input.\n> \n> Note that this is completely untested by me, as all of my AIX boxen have\n> gone away in the past few months (yay!).\n\nI did a re-build and test. Everything works fine.\n\nTested-by: Tor Arntsen \n\n-Tor\n\n-- >8 --\nSubject: [PATCH] Makefile: default pager on AIX to \"more\"\n\nAIX doesn't ship with \"less\" by default, and their \"more\" is\nmore featureful than average, so the latter is a more\nsensible choice. People who really want less can set the\ncompile-time option themselves, or users can set $PAGER.\n\nSigned-off-by: Jeff King \n---\n Makefile | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 34b7dd5..6ad0aca 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD)\n \tHAVE_PATHS_H = YesPlease\n endif\n ifeq ($(uname_S),AIX)\n+\tDEFAULT_PAGER = more\n \tNO_STRCASESTR=YesPlease\n \tNO_MEMMEM = YesPlease\n \tNO_MKDTEMP = YesPlease\n-- 1.7.1.514.g71ed8 \n"},{"id":"143417","messageId":"AANLkTimI0DAHWzAPYRcui3h7KDvf1NFQBx0p4LdlYHTJ@mail.gmail.com","threadId":"24037","inReplyTo":"4C10AF43.5050708@spacetec.no","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Dario Rodriguez","fromEmail":"soft.d4rio@gmail.com","sentAt":"2010-06-10T11:31:38Z","receivedAt":"2010-06-10T11:31:38Z","isPatch":true,"sender":{"key":"soft.d4rio@gmail.com","avatar":"https://gravatar.com/avatar/9bca8035ad520a6fc0eb66e5126adbe45992c8388c05d1f7952bd4793eb82c22?d=mp&s=160"},"body":"On Thu, Jun 10, 2010 at 6:24 AM, Tor Arntsen wrote:\n> On 06/10/10 10:59, Jeff King wrote:\n>> On Thu, Jun 10, 2010 at 10:48:31AM +0200, Tor Arntsen wrote:\n>>\n>>> That looks good to me.\n>>\n>> OK, here it is with a commit message. Other systems might want the same,\n>> I guess (Solaris, IRIX?). I'm cc'ing Brandon, who might have some input.\n>>\n>> Note that this is completely untested by me, as all of my AIX boxen have\n>> gone away in the past few months (yay!).\n>\n> I did a re-build and test. Everything works fine.\n>\n> Tested-by: Tor Arntsen \n>\n> -Tor\n>\n> -- >8 --\n> Subject: [PATCH] Makefile: default pager on AIX to \"more\"\n>\n> AIX doesn't ship with \"less\" by default, and their \"more\" is\n> more featureful than average, so the latter is a more\n> sensible choice.  People who really want less can set the\n> compile-time option themselves, or users can set $PAGER.\n>\n> Signed-off-by: Jeff King \n> ---\n>  Makefile |    1 +\n>  1 files changed, 1 insertions(+), 0 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 34b7dd5..6ad0aca 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD)\n>        HAVE_PATHS_H = YesPlease\n>  endif\n>  ifeq ($(uname_S),AIX)\n> +       DEFAULT_PAGER = more\n>        NO_STRCASESTR=YesPlease\n>        NO_MEMMEM = YesPlease\n>        NO_MKDTEMP = YesPlease\n> -- 1.7.1.514.g71ed8\n>\n\nIt's ok for me, I will test it on AIX today, if I've some time.\n\nCheers,\nDario\n"},{"id":"143442","messageId":"7vy6enlyy6.fsf@alter.siamese.dyndns.org","threadId":"24037","inReplyTo":"20100610085952.GA8269@coredump.intra.peff.net","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-10T14:55:13Z","receivedAt":"2010-06-10T14:55:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Subject: [PATCH] Makefile: default pager on AIX to \"more\"\n>\n> AIX doesn't ship with \"less\" by default, and their \"more\" is\n> more featureful than average, so the latter is a more\n> sensible choice. People who really want less can set the\n> compile-time option themselves, or users can set $PAGER.\n>\n> Signed-off-by: Jeff King \n> ---\n> Makefile | 1 +\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 34b7dd5..6ad0aca 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD)\n> \tHAVE_PATHS_H = YesPlease\n> endif\n> ifeq ($(uname_S),AIX)\n> +\tDEFAULT_PAGER = more\n> \tNO_STRCASESTR=YesPlease\n> \tNO_MEMMEM = YesPlease\n> \tNO_MKDTEMP = YesPlease\n\nA very sensible approach. Thanks.\n"},{"id":"143758","messageId":"gJV0lM_e77LzoiHR7moWdAApSZ7yI38lZ-w8kZwc97unWqtBc94nfg@cipher.nrlssc.navy.mil","threadId":"24037","inReplyTo":"20100610085952.GA8269@coredump.intra.peff.net","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2010-06-15T16:11:35Z","receivedAt":"2010-06-15T16:11:35Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"On 06/10/2010 03:59 AM, Jeff King wrote:\n> On Thu, Jun 10, 2010 at 10:48:31AM +0200, Tor Arntsen wrote:\n> \n>> That looks good to me.\n> \n> OK, here it is with a commit message. Other systems might want the same,\n> I guess (Solaris, IRIX?). I'm cc'ing Brandon, who might have some input.\n\nYes, I currently set DEFAULT_PAGER to 'more' in my config.mak file\non both of these platforms. The 'more' on IRIX is decent (it can go\nbackwards), but the 'more' on Solaris sucks. I've seen 'less' on some\nnewer versions of Solaris. Is it a standard component yet?\n\nSo, I think it's appropriate to set DEFAULT_PAGER on IRIX. There can't\nbe many users anyway. It's probably appropriate to set it on Solaris\ntoo, if 'less' is not a commonly installed component on modern systems.\nI wonder how surprised existing git users will be, for those on Solaris\nplatforms that have 'less' installed, when Solaris's crappy 'more'\nbecomes their pager.\n\nActually, there is a 'more' in /usr/xpg4/bin that is much better, but\nit is not being used when DEFAULT_PAGER is set to 'more'. Junio\ncreated the SANE_TOOL_PATH hack to add this additional path to the\nsearch path, but it is only implemented in git-sh-setup, so it only\nhas effect for git scripts. Maybe it should be added to setup_path().\n\nBut, I also think it would be nice if git fell back to the 'cat'\nbehavior when it fails to spawn the pager, because the following error\nis not very informative:\n\n casey@ # git log\n sh: less: not found\n Broken Pipe\n\n-brandon\n\n\n> Note that this is completely untested by me, as all of my AIX boxen have\n> gone away in the past few months (yay!).\n> \n> -- >8 --\n> Subject: [PATCH] Makefile: default pager on AIX to \"more\"\n> \n> AIX doesn't ship with \"less\" by default, and their \"more\" is\n> more featureful than average, so the latter is a more\n> sensible choice. People who really want less can set the\n> compile-time option themselves, or users can set $PAGER.\n> \n> Signed-off-by: Jeff King \n> ---\n> Makefile | 1 +\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n> \n> diff --git a/Makefile b/Makefile\n> index 34b7dd5..6ad0aca 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -930,6 +930,7 @@ ifeq ($(uname_S),NetBSD)\n> \tHAVE_PATHS_H = YesPlease\n> endif\n> ifeq ($(uname_S),AIX)\n> +\tDEFAULT_PAGER = more\n> \tNO_STRCASESTR=YesPlease\n> \tNO_MEMMEM = YesPlease\n> \tNO_MKDTEMP = YesPlease\n"},{"id":"143761","messageId":"4C17AB38.3060705@spacetec.no","threadId":"24037","inReplyTo":"gJV0lM_e77LzoiHR7moWdAApSZ7yI38lZ-w8kZwc97unWqtBc94nfg@cipher.nrlssc.navy.mil","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Tor Arntsen","fromEmail":"tor@spacetec.no","sentAt":"2010-06-15T16:32:56Z","receivedAt":"2010-06-15T16:32:56Z","isPatch":true,"sender":{"key":"tor@spacetec.no","avatar":null},"body":"On 06/15/10 18:11, Brandon Casey wrote:\n\n> Yes, I currently set DEFAULT_PAGER to 'more' in my config.mak file\n> on both of these platforms. The 'more' on IRIX is decent (it can go\n> backwards), but the 'more' on Solaris sucks. I've seen 'less' on some\n> newer versions of Solaris. Is it a standard component yet?\n\n'less' appears to be standard on Solaris 10 (5.10). My older 5.8 box\nis offline so I can't check that one. AFAIK the difference between\nSolaris 8 and Solaris 10 is pretty big so I suspect there's no 'less' \nthere but I can't say for certain. I could try to fire up that old \nbox and check though.\n\n-Tor\n"},{"id":"143804","messageId":"AANLkTinb7eRrRRYxxRd3BbNfBwRYAVFTNJ7z8oklNvIs@mail.gmail.com","threadId":"24037","inReplyTo":"gJV0lM_e77LzoiHR7moWdAApSZ7yI38lZ-w8kZwc97unWqtBc94nfg@cipher.nrlssc.navy.mil","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Nazri Ramliy","fromEmail":"ayiehere@gmail.com","sentAt":"2010-06-16T01:34:52Z","receivedAt":"2010-06-16T01:34:52Z","isPatch":true,"sender":{"key":"ayiehere@gmail.com","avatar":"https://avatars.githubusercontent.com/u/164756?v=4"},"body":"On Wed, Jun 16, 2010 at 12:11 AM, Brandon Casey\n wrote:\n> But, I also think it would be nice if git fell back to the 'cat'\n> behavior when it fails to spawn the pager, because the following error\n> is not very informative:\n\nFalling back to 'cat' is nice except when you are on a remote machine\nwith very bad latency.\n\nnazri\n"},{"id":"143811","messageId":"20100616062814.GA13481@sigill.intra.peff.net","threadId":"24037","inReplyTo":"gJV0lM_e77LzoiHR7moWdAApSZ7yI38lZ-w8kZwc97unWqtBc94nfg@cipher.nrlssc.navy.mil","subject":"Re: [PATCH/RFC] Fix for default pager","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-06-16T06:28:15Z","receivedAt":"2010-06-16T06:28:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 15, 2010 at 11:11:35AM -0500, Brandon Casey wrote:\n\n> So, I think it's appropriate to set DEFAULT_PAGER on IRIX. There can't\n> be many users anyway. It's probably appropriate to set it on Solaris\n> too, if 'less' is not a commonly installed component on modern systems.\n> I wonder how surprised existing git users will be, for those on Solaris\n> platforms that have 'less' installed, when Solaris's crappy 'more'\n> becomes their pager.\n\nI'm a little worried about that, too. On the other hand, wouldn't people\nwho actually care about less have set PAGER already, to use it for\nthings like \"man\"?\n\n> But, I also think it would be nice if git fell back to the 'cat'\n> behavior when it fails to spawn the pager, because the following error\n> is not very informative:\n> \n> casey@ # git log\n> sh: less: not found\n> Broken Pipe\n\nThe pager command is executed by the shell these days. Perhaps we should\nsimply set DEFAULT_PAGER on these platforms to \"less || more || cat\",\nwhich seems to work from my simple tests. If that is too hack-ish (e.g.,\nwe really care about \"does less exist\", not \"did it fail\"), we can do a\nmore invasive patch (or even provide a \"git-pager\" shell script helper\nto do a more thorough job).\n\n-Peff\n"}]}