Re: [PATCH v2 06/14] apply: only pass required data to gitdiff_* functions
From: Johannes Schindelin <hidden>
Date: 2019-07-05 18:51:05
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...
quoted hunk ↗ jump to hunk
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