Subject: Re: [PATCHv4 09/17] submodule.c: convert show_submodule_summary to use emit_line_fmt

From: Junio C Hamano <[email-masked]>
Date: Tue, 23 May 2017 14:59:16 +0900

Stefan Beller <[email-masked]> writes:

> diff --git a/submodule.c b/submodule.c
> index d3299e29c0..428c996c97 100644
> --- a/submodule.c
> +++ b/submodule.c
> ...
> @@ -547,15 +543,16 @@ void show_submodule_inline_diff(FILE *f, const char *path,
>  	if (right)
>  		new = two;
>  
> -	fflush(f);
>  	cp.git_cmd = 1;
>  	cp.dir = path;
> -	cp.out = dup(fileno(f));
> +	cp.out = -1;
>  	cp.no_stdin = 1;
>  
>  	/* TODO: other options may need to be passed here. */
>  	argv_array_push(&cp.args, "diff");
> -	argv_array_pushf(&cp.args, "--line-prefix=%s", line_prefix);
> +	if (o->use_color)
> +		argv_array_push(&cp.args, "--color=always");
> +	argv_array_pushf(&cp.args, "--line-prefix=%s", diff_line_prefix(o));

This makes me wonder if we also need to explicitly decline coloring
when o->use_color is not set.  After all, even if configuration in
the submodule's config file says diff.color=never, we will enable
the color with this codepath (because the user explicitly asked to
use the color in the top-level), so we should do the same for the
opposite case where the config says yes/auto if the user said no at
the top-level, no?