On Mon, Apr 25, 2016 at 9:32 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
> <christian.couder@xxxxxxxxx> wrote:
>> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
>> ---
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> @@ -4670,6 +4670,28 @@ static int option_parse_directory(const struct option *opt,
>> +static void init_apply_state(struct apply_state *state, const char *prefix_)
>> +{
>> + memset(state, 0, sizeof(*state));
>> + state->prefix = prefix_;
>> + state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
>> + state->apply = 1;
>> + state->line_termination = '\n';
>> + state->p_value = 1;
>> + state->p_context = UINT_MAX;
>> + state->squelch_whitespace_errors = 5;
>> + state->ws_error_action = warn_on_ws_error;
>> + state->ws_ignore_action = ignore_ws_none;
>> + state->linenr = 1;
>> + strbuf_init(&state->root, 0);
>> +
>> + git_apply_config();
>> + if (apply_default_whitespace)
>> + parse_whitespace_option(state, apply_default_whitespace);
>> + if (apply_default_ignorewhitespace)
>> + parse_ignorewhitespace_option(state, apply_default_ignorewhitespace);
>> +}
>
> Minor:
>
> If factoring out this code from cmd_apply() into init_apply_state()
> was done as a preparatory patch before introduction of 'apply_state',
> then each new 'state->foo=...' line would already be at its final
> location when added by its respective patch.
Yeah I agree. So I did something like that.
And by the way while doing it I saw that Junio also suggested a similar change.
The little difference with what you and Junio suggest is that the
patch that creates init_apply_state() is just after the patch that
introduces 'struct apply_state'. I think it is a bit cleaner this way,
as if I create init_apply_state() first, then I would have to change
its arguments in the next patch that introduces 'struct apply_state'.
> Doing so would also provide an obvious opportunity to name the
> 'prefix' argument to init_apply_state() "prefix" rather than the odd
> "prefix_".
Yeah, I did that too.
Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html