Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> When submitting a revised a patch series, the --range-diff option embeds
> a range-diff in the cover letter showing changes since the previous
> version of the patch series. The argument to --range-diff is a simple
> revision naming the tip of the previous series, which works fine if the
> previous and current versions of the patch series share a common base.
>
> However, it fails if the revision ranges of the old and new versions of
> the series are disjoint. To address this shortcoming, extend
> --range-diff to also accept an explicit revision range for the previous
> series. For example:
>
> git format-patch --cover-letter --range-diff=v1~3..v1 -3 v2
>
> Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
> ---
Makes sense. Even though a single "common rev" would serve as a
feature to discourage rebasing done "just to catch up" without a
good reason, it is a good idea to give an escape hatch like this
to support a case where rebasing is the right thing to do.
> static void infer_range_diff_ranges(struct strbuf *r1,
> struct strbuf *r2,
> const char *prev,
> + struct commit *origin,
> struct commit *head)
> {
> const char *head_oid = oid_to_hex(&head->object.oid);
>
> - strbuf_addf(r1, "%s..%s", head_oid, prev);
> - strbuf_addf(r2, "%s..%s", prev, head_oid);
I thought "git range-diff" also took the three-dot notation as a
short-hand but there is no point using that in this application,
which wants the RHS and the LHS range in separate output variables.
> + if (!strstr(prev, "..")) {
> + strbuf_addf(r1, "%s..%s", head_oid, prev);
> + strbuf_addf(r2, "%s..%s", prev, head_oid);
> + } else if (!origin) {
> + die(_("failed to infer range-diff ranges"));
> + } else {
> + strbuf_addstr(r1, prev);
> + strbuf_addf(r2, "%s..%s",
> + oid_to_hex(&origin->object.oid), head_oid);
Interesting.
I actually would feel better to see the second range for the normal
case to be computed exactly the same way, i.e.
int prev_is_range = strstr(prev, "..");
if (prev_is_range)
strbuf_addstr(r1, prev);
else
strbuf_addf(r1, "%s..%s", head_oid, prev);
if (origin)
strbuf_addf(r2, "%s..%s", oid_to_hex(&origin->object.oid, head_oid);
else if (prev_is_range)
die("cannot figure out the origin of new series");
else {
warning("falling back to use '%s' as the origin of new series", prev);
strbuf_addf(r2, "%s..%s", prev, head_oid);
}
because origin..HEAD is always the set of the "new" series, no
matter what "prev" the user chooses to compare that series against,
when there is a single "origin".
