Re: [PATCH/RFC] Fix for default pager

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Jun 8, 2010, 06:12 UTC
Message-ID: <4C0DDF53.5090805@viscovery.net>
In-Reply-To: <20100608052929.GA15156@coredump.intra.peff.net>

Am 6/8/2010 7:29, schrieb Jeff King:

Show 29 quoted lines

> On Mon, Jun 07, 2010 at 08:58:08PM -0300, Dario Rodriguez wrote:
>> +#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?

Yes and no.

Coding DOS magic numbers is bad because you could wrapped a real pager in a shell script (even on Windows).

These days, start_command() is able to report ENOENT if it cannot run a program because it does not exist. However, right in the case of git_pager() this capability is disabled because it sets this magic preexec_cb that waits for data in the child process before it execs the real pager.

If we could get rid of preexec_cb, we could make this work much more pleasantly. pager_preexec waits for data on stdin; Linus added it to work around a faulty 'less'. Do we still need it?

-- Hannes