Re: git diff: support "-U" and "--unified" options properly

From: Linus Torvalds <torvalds@osdl.org>
Date: May 15, 2006, 00:58 UTC
Message-ID: <Pine.LNX.4.64.0605141745410.3866@g5.osdl.org>
In-Reply-To: <7vbqu0yvda.fsf@assigned-by-dhcp.cox.net>

On Sun, 14 May 2006, Junio C Hamano wrote:

Show 5 quoted lines

> 
> I am not either after seeing your numbers and trying them
> myself.  I was going to look at it myself over the weekend, but
> I ended up spending most of the time migrating my environment,
> so no progress on that front yet.  Sorry.

I was looking at it, and I just suspect that "grep" is very optimized.

At the same time, the built-in one had many good features, notably that you could grep the cached copy and from a specific version. So I think it makes sense.

I have this suspicion that the best solution is to just handle the "grep against current tree" separately, and even make that something that only gets enabled on UNIX - I assume that trying to execve() grep externally would suck on windows again.

So I would actually assume that the solution is to simply make grep_cache() have a simple

	#ifdef __unix__
		if (!cached) {
			hit = external_grep(opt, paths, cached);
			if (hit >= 0)
				return hit;
		}
	#endif

at the top, so that we'd have the best of both worlds.

The "external_grep()" should look something like this:

	#define MAXARGS 1000

	static int external_grep(struct grep_opt *opt, const char **paths, int cached)
	{
		int nr;
		char *argv[MAXARGS];

		read_cache();
		argv[0] = "grep";
		argv[1] = "-e";
		argv[2] = opt->pattern;	/* whatever */
		argv[3] = "--";
		argc = 4;

		for (nr = 0; nr < active_nr; nr++) {
			struct cache_entry *ce = active_cache[nr];
			if (ce_stage(ce) || !S_ISREG(ntohl(ce->ce_mode)))
				continue;
			if (!pathspec_matches(paths, ce->name))
				continue;
			argv[argc++] = ce->name;
			if (argc < MAXARGS)
				continue;
			hit += exec_grep(argv, argc);
			argc = 4;
		}
		if (argc > 4)
			hit += exec_grep(argv, argc);
		return hit;
	}

and you're all done. Except for testing, and fixing my stupid bugs, which I'm too lazy to do.

The whole "exec_grep()" should basically be the same as "spawn_prog()". You get the idea.

Anybody?

		Linus