From: Erik Faye-Lund <kusmabite@googlemail.com>
Date: Tue, 08 Jun 2010 14:04:44 GMT
Subject: Re: [PATCH/RFC] Fix for default pager
Message-ID: <AANLkTikoHwupz8ZycLKko_gaZcsAnnXjlWIU4qpt-_9T@mail.gmail.com>
In-Reply-To: <AANLkTinxcrIV2TM966EkOC_crR0bHdNllEIdibz4gGjd@mail.gmail.com>

On Tue, Jun 8, 2010 at 2:24 PM, Dario Rodriguez <soft.d4rio@gmail.com> wrote:
> On Tue, Jun 8, 2010 at 2:29 AM, Jeff King <peff@peff.net> wrote:
>> On Mon, Jun 07, 2010 at 08:58:08PM -0300, Dario Rodriguez wrote:
>>
>>> Default pager was 'less' even when some systems such AIX and other basic
>>> or old systems do NOT have 'less' installed. In such case, git just
>>> does not display anything in pager-enabled functionalities such as 'git log'
>>> or 'git show', exiting with status 0.
>>>
>>> With this patch, git will not use DEFAULT_PAGER macro anymore, instead,
>>> git will look for 'less' and 'more' in the most common paths.
>>> If there is no pager, returns NULL as if it's 'cat'.
>>
>> Run-time pager detection seems like a reasonable goal, I guess, but...
>>
>>> -const char *git_pager(int stdout_is_tty)
>>> +static int is_executable(const char *name)
>>> +{
>>> +     struct stat st;
>>> +
>>> +     if (stat(name, &st) ||
>>> +         !S_ISREG(st.st_mode))
>>> +             return 0;
>>> +
>>> +#ifdef WIN32
>>> +{    /* cannot trust the executable bit, peek into the file instead */
>>> +     char buf[3] = { 0 };
>>> +     int n;
>>> +     int fd = open(name, O_RDONLY);
>>> +     st.st_mode &= ~S_IXUSR;
>>> +     if (fd >= 0) {
>>> +             n = read(fd, buf, 2);
>>> +             if (n == 2)
>>> +                     /* DOS executables start with "MZ" */
>>> +                     if (!strcmp(buf, "#!") || !strcmp(buf, "MZ"))
>>> +                             st.st_mode |= S_IXUSR;
>>> +             close(fd);
>>> +     }
>>> +}
>>> +#endif
>>> +     return st.st_mode & S_IXUSR;
>>> +}
>>> +
>>> +const char *git_pager(int stdout_is_tty)
>>>  {
>>> +     static const char *pager_bins[] =
>>> +             { "less", "more", NULL };
>>> +     static const char *common_binary_paths[] =
>>> +             { "/bin/","/usr/bin/","/usr/local/bin/",NULL };
>>
>> ...must we really add code with such ugliness as magic PATHs and DOS
>> magic numbers?
>>
>
> I copied the function 'is_executable' from 'help.c' so we already have
> such code... :p
>

How about just un-staticifying the version in help.c instead of
duplicating it, then? That way we'd have half the ugliness for this
purpose...

-- 
Erik "kusma" Faye-Lund