Merge branch 'es/format-patch-rangediff'
authorJunio C Hamano <gitster@pobox.com>
Mon, 17 Sep 2018 20:53:56 +0000 (13:53 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 17 Sep 2018 20:53:56 +0000 (13:53 -0700)
"git format-patch" learned a new "--range-diff" option to explain
the difference between this version and the previous attempt in
the cover letter (or after the tree-dashes as a comment).

* es/format-patch-rangediff:
format-patch: allow --range-diff to apply to a lone-patch
format-patch: add --creation-factor tweak for --range-diff
format-patch: teach --range-diff to respect -v/--reroll-count
format-patch: extend --range-diff to accept revision range
format-patch: add --range-diff option to embed diff in cover letter
range-diff: relieve callers of low-level configuration burden
range-diff: publish default creation factor
range-diff: respect diff_option.file rather than assuming 'stdout'

1  2 
builtin/log.c
builtin/range-diff.c
log-tree.c
range-diff.c
revision.h
t/t3206-range-diff.sh
diff --combined builtin/log.c
index df4201131284317948de938242c22d9862c0ffbb,f69b67b9cef050e1e5506d2b3a9f7c1f9c519a18..f09a5789f8a5a5252e9629b6c949adb2c365bf7e
@@@ -31,8 -31,8 +31,9 @@@
  #include "progress.h"
  #include "commit-slab.h"
  #include "repository.h"
 +#include "commit-reach.h"
  #include "interdiff.h"
+ #include "range-diff.h"
  
  #define MAIL_DEFAULT_WRAP 72
  
@@@ -1090,6 -1090,12 +1091,12 @@@ static void make_cover_letter(struct re
                fprintf_ln(rev->diffopt.file, "%s", rev->idiff_title);
                show_interdiff(rev, 0);
        }
+       if (rev->rdiff1) {
+               fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
+               show_range_diff(rev->rdiff1, rev->rdiff2,
+                               rev->creation_factor, 1, &rev->diffopt);
+       }
  }
  
  static const char *clean_message_id(const char *msg_id)
@@@ -1439,6 -1445,26 +1446,26 @@@ static const char *diff_title(struct st
        return sb->buf;
  }
  
+ 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);
+       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);
+       }
+ }
  int cmd_format_patch(int argc, const char **argv, const char *prefix)
  {
        struct commit *commit;
        struct progress *progress = NULL;
        struct oid_array idiff_prev = OID_ARRAY_INIT;
        struct strbuf idiff_title = STRBUF_INIT;
+       const char *rdiff_prev = NULL;
+       struct strbuf rdiff1 = STRBUF_INIT;
+       struct strbuf rdiff2 = STRBUF_INIT;
+       struct strbuf rdiff_title = STRBUF_INIT;
+       int creation_factor = -1;
  
        const struct option builtin_format_patch_options[] = {
                { OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
                OPT_CALLBACK(0, "interdiff", &idiff_prev, N_("rev"),
                             N_("show changes against <rev> in cover letter or single patch"),
                             parse_opt_object_name),
+               OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"),
+                          N_("show changes against <refspec> in cover letter or single patch")),
+               OPT_INTEGER(0, "creation-factor", &creation_factor,
+                           N_("percentage by which creation is weighted")),
                OPT_END()
        };
  
                numbered = 0;
  
        if (numbered && keep_subject)
 -              die (_("-n and -k are mutually exclusive."));
 +              die(_("-n and -k are mutually exclusive"));
        if (keep_subject && subject_prefix)
 -              die (_("--subject-prefix/--rfc and -k are mutually exclusive."));
 +              die(_("--subject-prefix/--rfc and -k are mutually exclusive"));
        rev.preserve_subject = keep_subject;
  
        argc = setup_revisions(argc, argv, &rev, &s_r_opt);
        if (argc > 1)
 -              die (_("unrecognized argument: %s"), argv[1]);
 +              die(_("unrecognized argument: %s"), argv[1]);
  
        if (rev.diffopt.output_format & DIFF_FORMAT_NAME)
                die(_("--name-only does not make sense"));
                                             _("Interdiff against v%d:"));
        }
  
+       if (creation_factor < 0)
+               creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
+       else if (!rdiff_prev)
+               die(_("--creation-factor requires --range-diff"));
+       if (rdiff_prev) {
+               if (!cover_letter && total != 1)
+                       die(_("--range-diff requires --cover-letter or single patch"));
+               infer_range_diff_ranges(&rdiff1, &rdiff2, rdiff_prev,
+                                       origin, list[0]);
+               rev.rdiff1 = rdiff1.buf;
+               rev.rdiff2 = rdiff2.buf;
+               rev.creation_factor = creation_factor;
+               rev.rdiff_title = diff_title(&rdiff_title, reroll_count,
+                                            _("Range-diff:"),
+                                            _("Range-diff against v%d:"));
+       }
        if (!signature) {
                ; /* --no-signature inhibits all signatures */
        } else if (signature && signature != git_version_string) {
                print_signature(rev.diffopt.file);
                total++;
                start_number--;
-               /* interdiff in cover-letter; omit from patches */
+               /* interdiff/range-diff in cover-letter; omit from patches */
                rev.idiff_oid1 = NULL;
+               rev.rdiff1 = NULL;
        }
        rev.add_signoff = do_signoff;
  
  done:
        oid_array_clear(&idiff_prev);
        strbuf_release(&idiff_title);
+       strbuf_release(&rdiff1);
+       strbuf_release(&rdiff2);
+       strbuf_release(&rdiff_title);
        return 0;
  }
  
diff --combined builtin/range-diff.c
index 0aa9bed41f35bf99784acd4abab52df415c5fb00,cc06e86a721731787be9ae4d8537dcc0d52aa678..96af5374937e5b8b99343e8d324e54e98fb11743
@@@ -11,35 -11,24 +11,24 @@@ N_("git range-diff [<options>] <base> <
  NULL
  };
  
- static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)
- {
-       return data;
- }
  int cmd_range_diff(int argc, const char **argv, const char *prefix)
  {
-       int creation_factor = 60;
+       int creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
        struct diff_options diffopt = { NULL };
        int simple_color = -1;
        struct option options[] = {
                OPT_INTEGER(0, "creation-factor", &creation_factor,
                            N_("Percentage by which creation is weighted")),
                OPT_BOOL(0, "no-dual-color", &simple_color,
 -                          N_("color both diff and diff-between-diffs")),
 +                          N_("use simple diff colors")),
                OPT_END()
        };
        int i, j, res = 0;
-       struct strbuf four_spaces = STRBUF_INIT;
        struct strbuf range1 = STRBUF_INIT, range2 = STRBUF_INIT;
  
        git_config(git_diff_ui_config, NULL);
  
        diff_setup(&diffopt);
-       diffopt.output_format = DIFF_FORMAT_PATCH;
-       diffopt.flags.suppress_diff_headers = 1;
-       diffopt.output_prefix = output_prefix_cb;
-       strbuf_addstr(&four_spaces, "    ");
-       diffopt.output_prefix_data = &four_spaces;
  
        argc = parse_options(argc, argv, NULL, options,
                             builtin_range_diff_usage, PARSE_OPT_KEEP_UNKNOWN |
                             options + ARRAY_SIZE(options) - 1, /* OPT_END */
                             builtin_range_diff_usage, 0);
  
-       if (simple_color < 1) {
-               if (!simple_color)
-                       /* force color when --dual-color was used */
-                       diffopt.use_color = 1;
-               diffopt.flags.dual_color_diffed_diffs = 1;
-       }
+       /* force color when --dual-color was used */
+       if (!simple_color)
+               diffopt.use_color = 1;
  
        if (argc == 2) {
                if (!strstr(argv[0], ".."))
        }
  
        res = show_range_diff(range1.buf, range2.buf, creation_factor,
-                             &diffopt);
+                             simple_color < 1, &diffopt);
  
        strbuf_release(&range1);
        strbuf_release(&range2);
-       strbuf_release(&four_spaces);
  
        return res;
  }
diff --combined log-tree.c
index f482f47b83dc3e2a999e00c93ac089fc6de5a6b9,cec983a4610649e3a8a80d70865549f48f645bea..e5b353ea2d587c091b841e92f7ddcde4c4266a1a
@@@ -16,6 -16,7 +16,7 @@@
  #include "line-log.h"
  #include "help.h"
  #include "interdiff.h"
+ #include "range-diff.h"
  
  static struct decoration name_decoration = { "object names" };
  static int decoration_loaded;
@@@ -93,7 -94,7 +94,7 @@@ static int add_ref_decoration(const cha
  
        if (starts_with(refname, git_replace_ref_base)) {
                struct object_id original_oid;
 -              if (!check_replace_refs)
 +              if (!read_replace_refs)
                        return 0;
                if (get_oid_hex(refname + strlen(git_replace_ref_base),
                                &original_oid)) {
@@@ -751,6 -752,20 +752,20 @@@ void show_log(struct rev_info *opt
  
                memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
        }
+       if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {
+               struct diff_queue_struct dq;
+               memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));
+               DIFF_QUEUE_CLEAR(&diff_queued_diff);
+               next_commentary_block(opt, NULL);
+               fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);
+               show_range_diff(opt->rdiff1, opt->rdiff2,
+                               opt->creation_factor, 1, &opt->diffopt);
+               memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
+       }
  }
  
  int log_tree_diff_flush(struct rev_info *opt)
diff --combined range-diff.c
index 3e9b9844012dac7fb4a0e80451e0d459da9a820e,3dd2edda0176b326d5980cbe1e37f3a8eec6a2f7..60edb2f518d6d696b8091ef6246791cd730f326f
@@@ -38,14 -38,6 +38,14 @@@ static int read_patches(const char *ran
  
        argv_array_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
                        "--reverse", "--date-order", "--decorate=no",
 +                      /*
 +                       * Choose indicators that are not used anywhere
 +                       * else in diffs, but still look reasonable
 +                       * (e.g. will not be confusing when debugging)
 +                       */
 +                      "--output-indicator-new=>",
 +                      "--output-indicator-old=<",
 +                      "--output-indicator-context=#",
                        "--no-abbrev-commit", range,
                        NULL);
        cp.out = -1;
@@@ -90,7 -82,6 +90,7 @@@
                        strbuf_addch(&buf, '\n');
                        if (!util->diff_offset)
                                util->diff_offset = buf.len;
 +                      strbuf_addch(&buf, ' ');
                        strbuf_addbuf(&buf, &line);
                } else if (in_header) {
                        if (starts_with(line.buf, "Author: ")) {
                         * we are not interested.
                         */
                        continue;
 -              else
 +              else if (line.buf[0] == '>') {
 +                      strbuf_addch(&buf, '+');
 +                      strbuf_add(&buf, line.buf + 1, line.len - 1);
 +              } else if (line.buf[0] == '<') {
 +                      strbuf_addch(&buf, '-');
 +                      strbuf_add(&buf, line.buf + 1, line.len - 1);
 +              } else if (line.buf[0] == '#') {
 +                      strbuf_addch(&buf, ' ');
 +                      strbuf_add(&buf, line.buf + 1, line.len - 1);
 +              } else {
 +                      strbuf_addch(&buf, ' ');
                        strbuf_addbuf(&buf, &line);
 +              }
  
                strbuf_addch(&buf, '\n');
                util->diffsize++;
@@@ -343,7 -323,7 +343,7 @@@ static void output_pair_header(struct d
        }
        strbuf_addf(buf, "%s\n", color_reset);
  
-       fwrite(buf->buf, buf->len, 1, stdout);
+       fwrite(buf->buf, buf->len, 1, diffopt->file);
  }
  
  static struct userdiff_driver no_func_name = {
@@@ -429,8 -409,14 +429,14 @@@ static void output(struct string_list *
        strbuf_release(&dashes);
  }
  
+ static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)
+ {
+       return data;
+ }
  int show_range_diff(const char *range1, const char *range2,
-                   int creation_factor, struct diff_options *diffopt)
+                   int creation_factor, int dual_color,
+                   struct diff_options *diffopt)
  {
        int res = 0;
  
                res = error(_("could not parse log for '%s'"), range2);
  
        if (!res) {
+               struct diff_options opts;
+               struct strbuf indent = STRBUF_INIT;
+               memcpy(&opts, diffopt, sizeof(opts));
+               opts.output_format = DIFF_FORMAT_PATCH;
+               opts.flags.suppress_diff_headers = 1;
+               opts.flags.dual_color_diffed_diffs = dual_color;
+               opts.output_prefix = output_prefix_cb;
+               strbuf_addstr(&indent, "    ");
+               opts.output_prefix_data = &indent;
+               diff_setup_done(&opts);
                find_exact_matches(&branch1, &branch2);
                get_correspondences(&branch1, &branch2, creation_factor);
-               output(&branch1, &branch2, diffopt);
+               output(&branch1, &branch2, &opts);
+               strbuf_release(&indent);
        }
  
        string_list_clear(&branch1, 1);
diff --combined revision.h
index 91bc180e37dc7d7810c0ab3e883e26cbe851385a,7106b89e9ea9884f4604a9b690911d7585c300bc..2b30ac270d9295e00641b08483fe07cd44dc51f6
@@@ -1,7 -1,6 +1,7 @@@
  #ifndef REVISION_H
  #define REVISION_H
  
 +#include "commit.h"
  #include "parse-options.h"
  #include "grep.h"
  #include "notes.h"
@@@ -82,11 -81,6 +82,11 @@@ struct rev_info 
         */
        int rev_input_given;
  
 +      /*
 +       * Whether we read from stdin due to the --stdin option.
 +       */
 +      int read_from_stdin;
 +
        /* topo-sort */
        enum rev_sort_order sort_order;
  
        const struct object_id *idiff_oid2;
        const char *idiff_title;
  
+       /* range-diff */
+       const char *rdiff1;
+       const char *rdiff2;
+       int creation_factor;
+       const char *rdiff_title;
        /* commit counts */
        int count_left;
        int count_right;
        struct revision_sources *sources;
  };
  
 -extern int ref_excluded(struct string_list *, const char *path);
 +int ref_excluded(struct string_list *, const char *path);
  void clear_ref_exclusion(struct string_list **);
  void add_ref_exclusion(struct string_list **, const char *exclude);
  
@@@ -263,39 -263,39 +269,39 @@@ struct setup_revision_opt 
        unsigned revarg_opt;
  };
  
 -extern void init_revisions(struct rev_info *revs, const char *prefix);
 -extern int setup_revisions(int argc, const char **argv, struct rev_info *revs,
 -                         struct setup_revision_opt *);
 -extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
 -                             const struct option *options,
 -                             const char * const usagestr[]);
 +void init_revisions(struct rev_info *revs, const char *prefix);
 +int setup_revisions(int argc, const char **argv, struct rev_info *revs,
 +                  struct setup_revision_opt *);
 +void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
 +                      const struct option *options,
 +                      const char * const usagestr[]);
  #define REVARG_CANNOT_BE_FILENAME 01
  #define REVARG_COMMITTISH 02
 -extern int handle_revision_arg(const char *arg, struct rev_info *revs,
 -                             int flags, unsigned revarg_opt);
 +int handle_revision_arg(const char *arg, struct rev_info *revs,
 +                      int flags, unsigned revarg_opt);
  
 -extern void reset_revision_walk(void);
 -extern int prepare_revision_walk(struct rev_info *revs);
 -extern struct commit *get_revision(struct rev_info *revs);
 -extern char *get_revision_mark(const struct rev_info *revs,
 -                             const struct commit *commit);
 -extern void put_revision_mark(const struct rev_info *revs,
 -                            const struct commit *commit);
 +void reset_revision_walk(void);
 +int prepare_revision_walk(struct rev_info *revs);
 +struct commit *get_revision(struct rev_info *revs);
 +char *get_revision_mark(const struct rev_info *revs,
 +                      const struct commit *commit);
 +void put_revision_mark(const struct rev_info *revs,
 +                     const struct commit *commit);
  
 -extern void mark_parents_uninteresting(struct commit *commit);
 -extern void mark_tree_uninteresting(struct tree *tree);
 +void mark_parents_uninteresting(struct commit *commit);
 +void mark_tree_uninteresting(struct tree *tree);
  
 -extern void show_object_with_name(FILE *, struct object *, const char *);
 +void show_object_with_name(FILE *, struct object *, const char *);
  
 -extern void add_pending_object(struct rev_info *revs,
 -                             struct object *obj, const char *name);
 -extern void add_pending_oid(struct rev_info *revs,
 -                          const char *name, const struct object_id *oid,
 -                          unsigned int flags);
 +void add_pending_object(struct rev_info *revs,
 +                      struct object *obj, const char *name);
 +void add_pending_oid(struct rev_info *revs,
 +                   const char *name, const struct object_id *oid,
 +                   unsigned int flags);
  
 -extern void add_head_to_pending(struct rev_info *);
 -extern void add_reflogs_to_pending(struct rev_info *, unsigned int flags);
 -extern void add_index_objects_to_pending(struct rev_info *, unsigned int flags);
 +void add_head_to_pending(struct rev_info *);
 +void add_reflogs_to_pending(struct rev_info *, unsigned int flags);
 +void add_index_objects_to_pending(struct rev_info *, unsigned int flags);
  
  enum commit_action {
        commit_ignore,
        commit_error
  };
  
 -extern enum commit_action get_commit_action(struct rev_info *revs,
 -                                          struct commit *commit);
 -extern enum commit_action simplify_commit(struct rev_info *revs,
 -                                        struct commit *commit);
 +enum commit_action get_commit_action(struct rev_info *revs,
 +                                   struct commit *commit);
 +enum commit_action simplify_commit(struct rev_info *revs,
 +                                 struct commit *commit);
  
  enum rewrite_result {
        rewrite_one_ok,
  
  typedef enum rewrite_result (*rewrite_parent_fn_t)(struct rev_info *revs, struct commit **pp);
  
 -extern int rewrite_parents(struct rev_info *revs, struct commit *commit,
 -      rewrite_parent_fn_t rewrite_parent);
 +int rewrite_parents(struct rev_info *revs,
 +                  struct commit *commit,
 +                  rewrite_parent_fn_t rewrite_parent);
  
  /*
   * The log machinery saves the original parent list so that
   * get_saved_parents() will transparently return commit->parents if
   * history simplification is off.
   */
 -extern struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);
 +struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);
  
  #endif
diff --combined t/t3206-range-diff.sh
index c94f9bf5ee12c1db38764ac6e51bbc011d3a2619,3d7a2d8a4d8191f93a70b2e3d4878537cd2c5564..88ebed1dfa59de4609b61f193fa3905c3238bf14
@@@ -133,52 -133,25 +133,64 @@@ test_expect_success 'changed message' 
            Z
            +    Also a silly comment here!
            +
 -          Zdiff --git a/file b/file
 -          Z--- a/file
 -          Z+++ b/file
 +          Z diff --git a/file b/file
 +          Z --- a/file
 +          Z +++ b/file
        3:  147e64e = 3:  b9cb956 s/11/B/
        4:  a63e992 = 4:  8add5f1 s/12/B/
        EOF
        test_cmp expected actual
  '
  
 +test_expect_success 'dual-coloring' '
 +      sed -e "s|^:||" >expect <<-\EOF &&
 +      :<YELLOW>1:  a4b3333 = 1:  f686024 s/5/A/<RESET>
 +      :<RED>2:  f51d370 <RESET><YELLOW>!<RESET><GREEN> 2:  4ab067d<RESET><YELLOW> s/4/A/<RESET>
 +      :    <REVERSE><CYAN>@@ -2,6 +2,8 @@<RESET>
 +      :     <RESET>
 +      :         s/4/A/<RESET>
 +      :     <RESET>
 +      :    <REVERSE><GREEN>+<RESET><BOLD>    Also a silly comment here!<RESET>
 +      :    <REVERSE><GREEN>+<RESET>
 +      :      diff --git a/file b/file<RESET>
 +      :      --- a/file<RESET>
 +      :      +++ b/file<RESET>
 +      :<RED>3:  0559556 <RESET><YELLOW>!<RESET><GREEN> 3:  b9cb956<RESET><YELLOW> s/11/B/<RESET>
 +      :    <REVERSE><CYAN>@@ -10,7 +10,7 @@<RESET>
 +      :      9<RESET>
 +      :      10<RESET>
 +      :    <RED> -11<RESET>
 +      :    <REVERSE><RED>-<RESET><FAINT;GREEN>+BB<RESET>
 +      :    <REVERSE><GREEN>+<RESET><BOLD;GREEN>+B<RESET>
 +      :      12<RESET>
 +      :      13<RESET>
 +      :      14<RESET>
 +      :<RED>4:  d966c5c <RESET><YELLOW>!<RESET><GREEN> 4:  8add5f1<RESET><YELLOW> s/12/B/<RESET>
 +      :    <REVERSE><CYAN>@@ -8,7 +8,7 @@<RESET>
 +      :    <CYAN> @@<RESET>
 +      :      9<RESET>
 +      :      10<RESET>
 +      :    <REVERSE><RED>-<RESET><FAINT> BB<RESET>
 +      :    <REVERSE><GREEN>+<RESET><BOLD> B<RESET>
 +      :    <RED> -12<RESET>
 +      :    <GREEN> +B<RESET>
 +      :      13<RESET>
 +      EOF
 +      git range-diff changed...changed-message --color --dual-color >actual.raw &&
 +      test_decode_color >actual <actual.raw &&
 +      test_cmp expect actual
 +'
 +
+ for prev in topic master..topic
+ do
+       test_expect_success "format-patch --range-diff=$prev" '
+               git format-patch --stdout --cover-letter --range-diff=$prev \
+                       master..unmodified >actual &&
+               grep "= 1: .* s/5/A" actual &&
+               grep "= 2: .* s/4/A" actual &&
+               grep "= 3: .* s/11/B" actual &&
+               grep "= 4: .* s/12/B" actual
+       '
+ done
  test_done