From: Jeff King <peff@peff.net>
Date: Sat, 20 Apr 2013 04:24:46 GMT
Subject: Re: [PATCH 6/6] grep: obey --textconv for the case rev:path
Message-ID: <20130420042445.GD24970@sigill.intra.peff.net>
In-Reply-To: <717ec305e9bd056a44b1da5cc478d314db2920e5.1366389739.git.git@drmicha.warpmail.net>

On Fri, Apr 19, 2013 at 06:44:49PM +0200, Michael J Gruber wrote:

> @@ -820,12 +820,13 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  	for (i = 0; i < argc; i++) {
>  		const char *arg = argv[i];
>  		unsigned char sha1[20];
> +		struct object_context oc;
>  		/* Is it a rev? */
> -		if (!get_sha1(arg, sha1)) {
> +		if (!get_sha1_with_context(arg, 0, sha1, &oc)) {
>  			struct object *object = parse_object_or_die(sha1, arg);
>  			if (!seen_dashdash)
>  				verify_non_filename(prefix, arg);
> -			add_object_array(object, arg, &list);
> +			add_object_array_with_context(object, arg, &list, xmemdupz(&oc, sizeof(struct object_context)));

Hrm. I'm not excited about the extra allocation here. Who frees it?

> +void add_object_array(struct object *obj, const char *name, struct object_array *array)
> +{
> +	add_object_array_with_mode(obj, name, array, S_IFINVALID);
> +}
> +
> +void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode)
> +{
> +	add_object_array_with_mode_context(obj, name, array, mode, NULL);
> +}
> +
> +void add_object_array_with_context(struct object *obj, const char *name, struct object_array *array, struct object_context *context)
> +{
> +	if (context)
> +		add_object_array_with_mode_context(obj, name, array, context->mode, context);
> +	else
> +		add_object_array_with_mode_context(obj, name, array, S_IFINVALID, context);
> +}

And this mass of almost-the-same functions is gross, too, especially
given that the object_context contains a mode itself.

Unfortunately, I'm not sure if I have a more pleasant suggestion. I seem
to recall wrestling with this issue during the last round, too.

-Peff