Sergey Organov <sorganov@xxxxxxxxx> writes:
> - if (rev->ignore_merges < 0) {
> - /* There was no "-m" variant on the command line */
> - rev->ignore_merges = 0;
> - if (!rev->first_parent_only && !rev->combine_merges) {
> - /* No "--first-parent", "-c", or "--cc" */
> - rev->combine_merges = 1;
> - rev->dense_combined_merges = 1;
> - }
> - }
> + rev_diff_merges_default_to_dense_combined(rev);
The name makes sense. "Unless otherwise specified, we do not ignore
merges. Further, when we are not doing first-parent traversal,
default to dense combined merges, unless told otherwise" is what the
code says, and the name of the helper captures it well.
> @@ -731,8 +723,7 @@ static void log_setup_revisions_tweak(struct rev_info *rev,
> if (!rev->diffopt.output_format && rev->combine_merges)
> rev->diffopt.output_format = DIFF_FORMAT_PATCH;
>
> - if (rev->first_parent_only && rev->ignore_merges < 0)
> - rev->ignore_merges = 0;
> + rev_diff_merges_first_parent_defaults_to_enable(rev);
This on the other hand is not so great a name. "When we are doing
first-parent traversal, do not exclude merge commits from being
shown" is what log_setup_revisions_tweak() does here. "default to
enable" is not clear at all what we are enabling.
Is your naming convention that "rev_diff_" is the common prefix?
What's the significance of "_diff" part? After all, these are about
tweaking the setting in the rev_info structure, so how about
revinfo_show_merges_in_dense_combined_by_default(rev);
revinfo_show_merges_by_default_with_first_parent(rev);
perhaps, using just "revinfo_" as the common prefix?
