On Wed, Jul 25, 2018 at 4:53 PM Junio C Hamano <gits...@pobox.com> wrote:
> Eric Sunshine <sunsh...@sunshineco.com> 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 [...]
> >
> > 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. [...]
>
> >  static void infer_range_diff_ranges(struct strbuf *r1,
> >  {
> > -     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.

Correct.

> 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".

I plan to submit a short series as incremental changes atop
es/format-patch-{interdiff,rangediff} to address the few review
comments[1] (including my own[2]), so I can make this change, as well.

[1]: 
https://public-inbox.org/git/CAPig+cSuYUYSPTuKx08wcmQM-G12_-W2T4BS07fA=6grm1b...@mail.gmail.com/T/#rfaf5a563e81b862bd8ed69232040056ab9a86dd8
[2]: 
https://public-inbox.org/git/CAPig+cSuYUYSPTuKx08wcmQM-G12_-W2T4BS07fA=6grm1b...@mail.gmail.com/

Reply via email to