Re: [PATCH v1 03/27] revision: factor out initialization of diff-merge related settings
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 3, 2020, 00:39 UTC
- Message-ID
- <xmqqmtyvk9jv.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20201108213838.4880-4-sorganov@gmail.com>
Sergey Organov <sorganov@gmail.com> writes:
Show 18 quoted lines
> Move initialization code related to diffing merges into new > init_diff_merge_revs() function. > > Signed-off-by: Sergey Organov <sorganov@gmail.com> > --- > 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.
Show 19 quoted lines
> 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;