On Tue, Jun 8, 2010 at 2:29 AM, Jeff King <peff@xxxxxxxx> 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
> Right now we fall back to just exec-ing "less". Could we instead just
> try to exec "less", if that fails then "more", and then finally "cat"?
>
is such a good idea but right now, 'git_pager' is not exec-ing, it's
just setting up a pager. If you set-up the pager based on wich one
fails in it's execution, you must avoid usage of this function, since
it will always return 'less' (or 'more...). What I posted is
transparent to any other function; 'git_pager' will be called
returning an existent, working pager, so the flow is the same, however
I like your proposal too, and should be considered.
> That would have almost the same effect and would be much simpler,
> wouldn't it? The exceptions I can think of are:
>
> - we would actually run "cat" in the final case, instead of optimizing
> it out.
>
Actually pager is being set to NULL if it's 'cat'... what's git doing
with a NULL pager?
> - "git var GIT_PAGER" wouldn't handle this automatically
>
> -Peff
>
Blessing,
Dario
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html