Re: [PATCH v1 03/27] revision: factor out initialization of diff-merge related settings
From: Junio C Hamano <hidden>
Date: 2020-12-03 00:39:45
Sergey Organov [off-list ref] writes:
quoted hunk
Move initialization code related to diffing merges into new init_diff_merge_revs() function. Signed-off-by: Sergey Organov <redacted> --- revision.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)diff --git a/revision.c b/revision.c index 739295bb9ff4..bc568fb79778 100644 --- a/revision.c +++ b/revision.c@@ -1805,6 +1805,8 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags, return 1; } +static void init_diff_merge_revs(struct rev_info *revs); +
It is not wrong per-se, but I do not see why we do not define the function here, not just forward-declare it like so.
quoted hunk
void repo_init_revisions(struct repository *r, struct rev_info *revs, const char *prefix)@@ -1813,7 +1815,7 @@ void repo_init_revisions(struct repository *r, revs->repo = r; revs->abbrev = DEFAULT_ABBREV; - revs->ignore_merges = -1; + init_diff_merge_revs(revs); revs->simplify_history = 1; revs->pruning.repo = r; revs->pruning.flags.recursive = 1;@@ -2153,6 +2155,10 @@ static void add_message_grep(struct rev_info *revs, const char *pattern) add_grep(revs, pattern, GREP_PATTERN_BODY); } +static void init_diff_merge_revs(struct rev_info *revs) { + revs->ignore_merges = -1; +}
Style. A brace that begins a function body begins its own line. In any case, I'd move this new definition before its sole caller; that way we do not even need an extra forward-declaration.
static int parse_diff_merge_opts(struct rev_info *revs, const char **argv) {
int argcount;
const char *optarg;