| Message ID | 20201108213838.4880-10-sorganov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-log: implement new --diff-merge options | expand |
On Sun, Nov 8, 2020 at 1:43 PM Sergey Organov <sorganov@gmail.com> wrote: > > For clarity, define public functions in the order they are called, to > make logic inter-dependencies easier to grok. You added diff-merges.[ch] earlier in this series. Why not just add them in the correct order initially instead of adding another patch later in the series? > > Signed-off-by: Sergey Organov <sorganov@gmail.com> > --- > diff-merges.c | 24 ++++++++++++++---------- > diff-merges.h | 7 +++---- > 2 files changed, 17 insertions(+), 14 deletions(-) > > diff --git a/diff-merges.c b/diff-merges.c > index bb08a92e3b36..8536941e0b56 100644 > --- a/diff-merges.c > +++ b/diff-merges.c > @@ -2,6 +2,10 @@ > > #include "revision.h" > > +/* > + * Public functions. They are in the order they are called. > + */ > + > void diff_merges_init_revs(struct rev_info *revs) { > revs->ignore_merges = -1; > } > @@ -44,16 +48,6 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv) { > return 1; > } > > -void diff_merges_setup_revs(struct rev_info *revs) > -{ > - if (revs->combine_merges && revs->ignore_merges < 0) > - revs->ignore_merges = 0; > - if (revs->ignore_merges < 0) > - revs->ignore_merges = 1; > - if (revs->combined_all_paths && !revs->combine_merges) > - die("--combined-all-paths makes no sense without -c or --cc"); > -} > - > void diff_merges_default_to_first_parent(struct rev_info *revs) { > if (revs->ignore_merges < 0) /* No -m */ > revs->ignore_merges = 0; > @@ -68,3 +62,13 @@ void diff_merges_default_to_dense_combined(struct rev_info *revs) { > } > } > } > + > +void diff_merges_setup_revs(struct rev_info *revs) > +{ > + if (revs->combine_merges && revs->ignore_merges < 0) > + revs->ignore_merges = 0; > + if (revs->ignore_merges < 0) > + revs->ignore_merges = 1; > + if (revs->combined_all_paths && !revs->combine_merges) > + die("--combined-all-paths makes no sense without -c or --cc"); > +} > diff --git a/diff-merges.h b/diff-merges.h > index 20b727bd734f..4b023c385d00 100644 > --- a/diff-merges.h > +++ b/diff-merges.h > @@ -7,11 +7,10 @@ void diff_merges_init_revs(struct rev_info *revs); > > int diff_merges_parse_opts(struct rev_info *revs, const char **argv); > > -void diff_merges_setup_revs(struct rev_info *revs); > - > -void diff_merges_default_to_dense_combined(struct rev_info *revs); > - > void diff_merges_default_to_first_parent(struct rev_info *revs); > > +void diff_merges_default_to_dense_combined(struct rev_info *revs); > + > +void diff_merges_setup_revs(struct rev_info *revs); > > #endif > -- > 2.25.1 >
Elijah Newren <newren@gmail.com> writes: > On Sun, Nov 8, 2020 at 1:43 PM Sergey Organov <sorganov@gmail.com> wrote: >> >> For clarity, define public functions in the order they are called, to >> make logic inter-dependencies easier to grok. > > You added diff-merges.[ch] earlier in this series. Why not just add > them in the correct order initially instead of adding another patch > later in the series? Well, I did consider it, but there are 2 issues that stopped me. First, I didn't want to rearrange functions as I move them from revision.c, to avoid mixed commit to simplify review, and second, I didn't want to rearrange them in the original to perform as little changes to the codebase as possible before isolating my work into diff-merges.[ch]. Hope it makes sense. Thanks, -- Sergey Organov
diff --git a/diff-merges.c b/diff-merges.c index bb08a92e3b36..8536941e0b56 100644 --- a/diff-merges.c +++ b/diff-merges.c @@ -2,6 +2,10 @@ #include "revision.h" +/* + * Public functions. They are in the order they are called. + */ + void diff_merges_init_revs(struct rev_info *revs) { revs->ignore_merges = -1; } @@ -44,16 +48,6 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv) { return 1; } -void diff_merges_setup_revs(struct rev_info *revs) -{ - if (revs->combine_merges && revs->ignore_merges < 0) - revs->ignore_merges = 0; - if (revs->ignore_merges < 0) - revs->ignore_merges = 1; - if (revs->combined_all_paths && !revs->combine_merges) - die("--combined-all-paths makes no sense without -c or --cc"); -} - void diff_merges_default_to_first_parent(struct rev_info *revs) { if (revs->ignore_merges < 0) /* No -m */ revs->ignore_merges = 0; @@ -68,3 +62,13 @@ void diff_merges_default_to_dense_combined(struct rev_info *revs) { } } } + +void diff_merges_setup_revs(struct rev_info *revs) +{ + if (revs->combine_merges && revs->ignore_merges < 0) + revs->ignore_merges = 0; + if (revs->ignore_merges < 0) + revs->ignore_merges = 1; + if (revs->combined_all_paths && !revs->combine_merges) + die("--combined-all-paths makes no sense without -c or --cc"); +} diff --git a/diff-merges.h b/diff-merges.h index 20b727bd734f..4b023c385d00 100644 --- a/diff-merges.h +++ b/diff-merges.h @@ -7,11 +7,10 @@ void diff_merges_init_revs(struct rev_info *revs); int diff_merges_parse_opts(struct rev_info *revs, const char **argv); -void diff_merges_setup_revs(struct rev_info *revs); - -void diff_merges_default_to_dense_combined(struct rev_info *revs); - void diff_merges_default_to_first_parent(struct rev_info *revs); +void diff_merges_default_to_dense_combined(struct rev_info *revs); + +void diff_merges_setup_revs(struct rev_info *revs); #endif
For clarity, define public functions in the order they are called, to make logic inter-dependencies easier to grok. Signed-off-by: Sergey Organov <sorganov@gmail.com> --- diff-merges.c | 24 ++++++++++++++---------- diff-merges.h | 7 +++---- 2 files changed, 17 insertions(+), 14 deletions(-)