Jeff King <peff@xxxxxxxx> writes:
> It's too late for "-m" to change semantics (we could do a long
> deprecation, but I don't see much point in doing so). But --diff-merges
> is definitely still changeable until we release v2.29. My resistance was
> mostly that I didn't want to complicate my series by adding new
> elements. But we could do something on top.
Attached is rather minimal incompatible change to --diff-merges that'd
allow extensions in the future, to get out of urge for the discussed
changes. I'm going to follow-up with actual improvements and I'm aware
it lacks documentation changes.
What do you think, is it OK to have something like this before v2.29?
And, by the way, what's approximate timeline to v2.29?
As for me, I'm not sure 'combined-all-paths' should be included and if
it should, is it a good enough name. In addition, do we need more
descriptive (additional) names for "c" and "cc" modes?
-- Sergey
>From b75e410797d4576e266d056ece16acf07e4b0116 Mon Sep 17 00:00:00 2001
From: Sergey Organov <sorganov@xxxxxxxxx>
Date: Tue, 4 Aug 2020 16:48:27 +0300
Subject: [PATCH RFC] revision: change "--diff-merges" option to require
parameter
Make --diff-merges require parameter having one of the following values:
off, all, c, cc, combined-all-paths
that are equivalents of passing the following separate options, respectively:
--no-diff-merges, -m, -c, -cc, --combined-all-paths
that, except --no-diff-merges, could be deprecated later.
This gives us single option that entirely defines how merge commits are to be
represented.
This patch is a preparation for supporting --diff-merges=<parent>, where
<parent> is parent number to provide diff against, that adds new essential
functionality and therefore is a separate change.
Signed-off-by: Sergey Organov <sorganov@xxxxxxxxx>
---
revision.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/revision.c b/revision.c
index 669bc856694f..dcdff59bc36a 100644
--- a/revision.c
+++ b/revision.c
@@ -2323,10 +2323,31 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
revs->diff = 1;
revs->diffopt.flags.recursive = 1;
revs->diffopt.flags.tree_in_recursive = 1;
- } else if (!strcmp(arg, "-m") || !strcmp(arg, "--diff-merges")) {
+ } else if ((argcount = parse_long_opt("diff-merges", argv, &optarg))) {
revs->ignore_merges = 0;
+ if (!strcmp(optarg, "off")) {
+ revs->ignore_merges = 1;
+ } else if (!strcmp(optarg, "all")) {
+ revs->diff = 0;
+ } else if (!strcmp(optarg, "c")) {
+ revs->diff = 1;
+ revs->dense_combined_merges = 0;
+ revs->combine_merges = 1;
+ } else if (!strcmp(optarg, "cc")) {
+ revs->diff = 1;
+ revs->dense_combined_merges = 1;
+ revs->combine_merges = 1;
+ } else if (!strcmp(optarg, "combined-all-paths")) {
+ revs->diff = 1;
+ revs->combined_all_paths = 1;
+ } else {
+ die("--diff-merges: unknown value '%s'.", optarg);
+ }
+ return argcount;
} else if (!strcmp(arg, "--no-diff-merges")) {
revs->ignore_merges = 1;
+ } else if (!strcmp(arg, "-m")) {
+ revs->ignore_merges = 0;
} else if (!strcmp(arg, "-c")) {
revs->diff = 1;
revs->dense_combined_merges = 0;
--
2.20.1
