Subject: Re: [PATCH v2 22/94] builtin/apply: move 'threeway' global into 'struct apply_state'

From: Junio C Hamano <[email-masked]>
Date: Thu, 12 May 2016 12:41:05 -0700

Christian Couder <[email-masked]> writes:

> To libify the apply functionality the 'threeway' variable should
> not be static and global to the file. Let's move it into
> 'struct apply_state'.
>
> Reviewed-by: Stefan Beller <[email-masked]>
> Signed-off-by: Christian Couder <[email-masked]>
> ---
>  builtin/apply.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 6216723..3650922 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -40,6 +40,7 @@ struct apply_state {
>  	int numstat;
>  
>  	int summary;
> +	int threeway;

This makes threeway look as if it is one of the cosmetic options
like stat/numstat/summary, doesn't it?  If anything, the blank
between numstat and summary should be moved below summary.  I'd
group knobs that affect "what is affected (check, check_index,
cached, etc.)", "how the application is done (allow-overlap,
threeway, in-reverse, etc.)" and "cosmetics" and place the ones in
the same group next to each other.