Hi Thomas,
On Fri, 5 Jul 2019, Thomas Gummerer wrote:
> Currently the 'gitdiff_*()' functions take 'struct apply_state' as
> parameter, even though they only needs the root, linenr and p_value
> from that struct.
>
> These functions are in the callchain of 'parse_git_header()', which we
> want to make more generally useful in a subsequent commit. To make
> that happen we only want to pass in the required data to
> 'parse_git_header()', and not the whole 'struct apply_state', and thus
> we want functions in the callchain of 'parse_git_header()' to only
> take arguments they really need.
This commit message follows the exact pattern of the previous patches (and
they were pretty convincing to me), but...
> diff --git a/apply.c b/apply.c
> index 3cd4e3d3b3..468f1d3fee 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -22,6 +22,12 @@
> #include "rerere.h"
> #include "apply.h"
>
> +struct parse_git_header_state {
> + struct strbuf *root;
> + int linenr;
> + int p_value;
> +};
> +
> static void git_apply_config(void)
> {
> git_config_get_string_const("apply.whitespace", &apply_default_whitespace);
> @@ -914,7 +920,7 @@ static int parse_traditional_patch(struct apply_state *state,
> return 0;
> }
>
> -static int gitdiff_hdrend(struct apply_state *state,
> +static int gitdiff_hdrend(struct parse_git_header_state *state,
... here we pass a different, newly-invented struct instead of passing the
relevant information explicitly.
My guess is that the code would look horribly verbose if we started
passing three instead of one parameter? If that is the case, I think a
little side note in the commit message might be warranted.
Ciao,
Dscho
