Re: [PATCH/RFC] Fix for default pager

[Jeff King <peff@peff.net>]

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?

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"?

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.

  - "git var GIT_PAGER" wouldn't handle this automatically

-Peff