Re: [PATCH 18/83] builtin/apply: move 'numstat' global into 'struct apply_state'

[Christian Couder <christian.couder@gmail.com>]

On Mon, Apr 25, 2016 at 11:40 PM, Stefan Beller <sbeller@google.com> wrote:
> On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>>  builtin/apply.c | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> index d90948a..16d78f9 100644
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
>> @@ -36,6 +36,9 @@ struct apply_state {
>>         /* --stat does just a diffstat, and doesn't actually apply */
>>         int diffstat;
>>
>> +       /* --numstat does numeric diffstat, and doesn't actually apply */
>> +       int numstat;
>> +
>>         /*
>>          *  --check turns on checking that the working tree matches the
>>          *    files that are being modified, but doesn't apply the patch
>> @@ -51,14 +54,12 @@ struct apply_state {
>>  };
>>
>>  /*
>> - *  --numstat does numeric diffstat, and doesn't actually apply
>>   *  --index-info shows the old and new index info for paths if available.
>>   */
>>  static int newfd = -1;
>>
>>  static int state_p_value = 1;
>>  static int p_value_known;
>> -static int numstat;
>>  static int summary;
>>  static int apply = 1;
>>  static int no_add;
>> @@ -4500,7 +4501,7 @@ static int apply_patch(struct apply_state *state,
>>         if (state->diffstat)
>>                 stat_patch_list(list);
>>
>> -       if (numstat)
>> +       if (state->numstat)
>>                 numstat_patch_list(list);
>>
>>         if (summary)
>> @@ -4598,7 +4599,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
>>                         N_("instead of applying the patch, output diffstat for the input")),
>>                 OPT_NOOP_NOARG(0, "allow-binary-replacement"),
>>                 OPT_NOOP_NOARG(0, "binary"),
>> -               OPT_BOOL(0, "numstat", &numstat,
>> +               OPT_BOOL(0, "numstat", &state.numstat,
>>                         N_("show number of added and deleted lines in decimal notation")),
>>                 OPT_BOOL(0, "summary", &summary,
>>                         N_("instead of applying the patch, output a summary for the input")),
>> @@ -4675,7 +4676,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
>>         }
>>         if (state.apply_with_reject)
>>                 apply = state.apply_verbosely = 1;
>> -       if (!force_apply && (state.diffstat || numstat || summary || state.check || fake_ancestor))
>> +       if (!force_apply && (state.diffstat || state.numstat || summary || state.check || fake_ancestor))
>
> Mental note: This patch is just doing a mechanical conversion, so it
> is fine to check for many "state.FOOs" here.
>
> However later we may want to move this out to a static oneliner like:
>
>     static int really_apply(state *s) {
>       return s->diffstat || s->numstat || ...;
>     }
>
> (with a better name obviously)

Yeah, this is another cleanup that could be done.
I added it to a list and will try to take care of it later.