Merge branch 'rs/blame-code-cleanup'
authorJunio C Hamano <gitster@pobox.com>
Fri, 17 Mar 2017 20:50:24 +0000 (13:50 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 17 Mar 2017 20:50:24 +0000 (13:50 -0700)
Code clean-up.

* rs/blame-code-cleanup:
blame: move blame_entry duplication to add_blame_entry()

1  2 
builtin/blame.c
diff --combined builtin/blame.c
index cffc62654084b025ffbdb947448fa86c81dc6442,701d9bb2661dcba4aaff66c4c78e4f5da39ef688..f7aa95f4babaab904443784eccdaa3b219bd2b08
@@@ -120,7 -120,7 +120,7 @@@ struct origin 
         */
        struct blame_entry *suspects;
        mmfile_t file;
 -      unsigned char blob_sha1[20];
 +      struct object_id blob_oid;
        unsigned mode;
        /* guilty gets set when shipping any suspects to the final
         * blame list instead of other commits
@@@ -154,8 -154,8 +154,8 @@@ static int diff_hunks(mmfile_t *file_a
   */
  int textconv_object(const char *path,
                    unsigned mode,
 -                  const unsigned char *sha1,
 -                  int sha1_valid,
 +                  const struct object_id *oid,
 +                  int oid_valid,
                    char **buf,
                    unsigned long *buf_size)
  {
        struct userdiff_driver *textconv;
  
        df = alloc_filespec(path);
 -      fill_filespec(df, sha1, sha1_valid, mode);
 +      fill_filespec(df, oid->hash, oid_valid, mode);
        textconv = get_textconv(df);
        if (!textconv) {
                free_filespec(df);
@@@ -188,16 -188,15 +188,16 @@@ static void fill_origin_blob(struct dif
  
                num_read_blob++;
                if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
 -                  textconv_object(o->path, o->mode, o->blob_sha1, 1, &file->ptr, &file_size))
 +                  textconv_object(o->path, o->mode, &o->blob_oid, 1, &file->ptr, &file_size))
                        ;
                else
 -                      file->ptr = read_sha1_file(o->blob_sha1, &type, &file_size);
 +                      file->ptr = read_sha1_file(o->blob_oid.hash, &type,
 +                                                 &file_size);
                file->size = file_size;
  
                if (!file->ptr)
                        die("Cannot read blob %s for path %s",
 -                          sha1_to_hex(o->blob_sha1),
 +                          oid_to_hex(&o->blob_oid),
                            o->path);
                o->file = *file;
        }
@@@ -509,17 -508,17 +509,17 @@@ static struct origin *get_origin(struc
   */
  static int fill_blob_sha1_and_mode(struct origin *origin)
  {
 -      if (!is_null_sha1(origin->blob_sha1))
 +      if (!is_null_oid(&origin->blob_oid))
                return 0;
        if (get_tree_entry(origin->commit->object.oid.hash,
                           origin->path,
 -                         origin->blob_sha1, &origin->mode))
 +                         origin->blob_oid.hash, &origin->mode))
                goto error_out;
 -      if (sha1_object_info(origin->blob_sha1, NULL) != OBJ_BLOB)
 +      if (sha1_object_info(origin->blob_oid.hash, NULL) != OBJ_BLOB)
                goto error_out;
        return 0;
   error_out:
 -      hashclr(origin->blob_sha1);
 +      oidclr(&origin->blob_oid);
        origin->mode = S_IFINVALID;
        return -1;
  }
@@@ -573,7 -572,7 +573,7 @@@ static struct origin *find_origin(struc
        if (!diff_queued_diff.nr) {
                /* The path is the same as parent */
                porigin = get_origin(sb, parent, origin->path);
 -              hashcpy(porigin->blob_sha1, origin->blob_sha1);
 +              oidcpy(&porigin->blob_oid, &origin->blob_oid);
                porigin->mode = origin->mode;
        } else {
                /*
                            p->status);
                case 'M':
                        porigin = get_origin(sb, parent, origin->path);
 -                      hashcpy(porigin->blob_sha1, p->one->oid.hash);
 +                      oidcpy(&porigin->blob_oid, &p->one->oid);
                        porigin->mode = p->one->mode;
                        break;
                case 'A':
@@@ -645,7 -644,7 +645,7 @@@ static struct origin *find_rename(struc
                if ((p->status == 'R' || p->status == 'C') &&
                    !strcmp(p->two->path, origin->path)) {
                        porigin = get_origin(sb, parent, p->one->path);
 -                      hashcpy(porigin->blob_sha1, p->one->oid.hash);
 +                      oidcpy(&porigin->blob_oid, &p->one->oid);
                        porigin->mode = p->one->mode;
                        break;
                }
  /*
   * Append a new blame entry to a given output queue.
   */
- static void add_blame_entry(struct blame_entry ***queue, struct blame_entry *e)
+ static void add_blame_entry(struct blame_entry ***queue,
+                           const struct blame_entry *src)
  {
+       struct blame_entry *e = xmalloc(sizeof(*e));
+       memcpy(e, src, sizeof(*e));
        origin_incref(e->suspect);
  
        e->next = **queue;
@@@ -760,21 -762,15 +763,15 @@@ static void split_blame(struct blame_en
                        struct blame_entry *split,
                        struct blame_entry *e)
  {
-       struct blame_entry *new_entry;
        if (split[0].suspect && split[2].suspect) {
                /* The first part (reuse storage for the existing entry e) */
                dup_entry(unblamed, e, &split[0]);
  
                /* The last part -- me */
-               new_entry = xmalloc(sizeof(*new_entry));
-               memcpy(new_entry, &(split[2]), sizeof(struct blame_entry));
-               add_blame_entry(unblamed, new_entry);
+               add_blame_entry(unblamed, &split[2]);
  
                /* ... and the middle part -- parent */
-               new_entry = xmalloc(sizeof(*new_entry));
-               memcpy(new_entry, &(split[1]), sizeof(struct blame_entry));
-               add_blame_entry(blamed, new_entry);
+               add_blame_entry(blamed, &split[1]);
        }
        else if (!split[0].suspect && !split[2].suspect)
                /*
        else if (split[0].suspect) {
                /* me and then parent */
                dup_entry(unblamed, e, &split[0]);
-               new_entry = xmalloc(sizeof(*new_entry));
-               memcpy(new_entry, &(split[1]), sizeof(struct blame_entry));
-               add_blame_entry(blamed, new_entry);
+               add_blame_entry(blamed, &split[1]);
        }
        else {
                /* parent and then me */
                dup_entry(blamed, e, &split[1]);
-               new_entry = xmalloc(sizeof(*new_entry));
-               memcpy(new_entry, &(split[2]), sizeof(struct blame_entry));
-               add_blame_entry(unblamed, new_entry);
+               add_blame_entry(unblamed, &split[2]);
        }
  }
  
@@@ -1309,7 -1299,7 +1300,7 @@@ static void find_copy_in_parent(struct 
                                continue;
  
                        norigin = get_origin(sb, parent, p->one->path);
 -                      hashcpy(norigin->blob_sha1, p->one->oid.hash);
 +                      oidcpy(&norigin->blob_oid, &p->one->oid);
                        norigin->mode = p->one->mode;
                        fill_origin_blob(&sb->revs->diffopt, norigin, &file_p);
                        if (!file_p.ptr)
@@@ -1459,14 -1449,15 +1450,14 @@@ static void pass_blame(struct scoreboar
                        porigin = find(sb, p, origin);
                        if (!porigin)
                                continue;
 -                      if (!hashcmp(porigin->blob_sha1, origin->blob_sha1)) {
 +                      if (!oidcmp(&porigin->blob_oid, &origin->blob_oid)) {
                                pass_whole_blame(sb, origin, porigin);
                                origin_decref(porigin);
                                goto finish;
                        }
                        for (j = same = 0; j < i; j++)
                                if (sg_origin[j] &&
 -                                  !hashcmp(sg_origin[j]->blob_sha1,
 -                                           porigin->blob_sha1)) {
 +                                  !oidcmp(&sg_origin[j]->blob_oid, &porigin->blob_oid)) {
                                        same = 1;
                                        break;
                                }
@@@ -1700,23 -1691,13 +1691,23 @@@ static void get_commit_info(struct comm
  }
  
  /*
 + * Write out any suspect information which depends on the path. This must be
 + * handled separately from emit_one_suspect_detail(), because a given commit
 + * may have changes in multiple paths. So this needs to appear each time
 + * we mention a new group.
 + *
   * To allow LF and other nonportable characters in pathnames,
   * they are c-style quoted as needed.
   */
 -static void write_filename_info(const char *path)
 +static void write_filename_info(struct origin *suspect)
  {
 +      if (suspect->previous) {
 +              struct origin *prev = suspect->previous;
 +              printf("previous %s ", oid_to_hex(&prev->commit->object.oid));
 +              write_name_quoted(prev->path, stdout, '\n');
 +      }
        printf("filename ");
 -      write_name_quoted(path, stdout, '\n');
 +      write_name_quoted(suspect->path, stdout, '\n');
  }
  
  /*
@@@ -1745,6 -1726,11 +1736,6 @@@ static int emit_one_suspect_detail(stru
        printf("summary %s\n", ci.summary.buf);
        if (suspect->commit->object.flags & UNINTERESTING)
                printf("boundary\n");
 -      if (suspect->previous) {
 -              struct origin *prev = suspect->previous;
 -              printf("previous %s ", oid_to_hex(&prev->commit->object.oid));
 -              write_name_quoted(prev->path, stdout, '\n');
 -      }
  
        commit_info_destroy(&ci);
  
@@@ -1765,7 -1751,7 +1756,7 @@@ static void found_guilty_entry(struct b
                       oid_to_hex(&suspect->commit->object.oid),
                       ent->s_lno + 1, ent->lno + 1, ent->num_lines);
                emit_one_suspect_detail(suspect, 0);
 -              write_filename_info(suspect->path);
 +              write_filename_info(suspect);
                maybe_flush_or_die(stdout, "stdout");
        }
        pi->blamed_lines += ent->num_lines;
@@@ -1889,7 -1875,7 +1880,7 @@@ static void emit_porcelain_details(stru
  {
        if (emit_one_suspect_detail(suspect, repeat) ||
            (suspect->commit->object.flags & MORE_THAN_ONE_PATH))
 -              write_filename_info(suspect->path);
 +              write_filename_info(suspect);
  }
  
  static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
        struct origin *suspect = ent->suspect;
        char hex[GIT_SHA1_HEXSZ + 1];
  
 -      sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
 +      oid_to_hex_r(hex, &suspect->commit->object.oid);
        printf("%s %d %d %d\n",
               hex,
               ent->s_lno + 1,
@@@ -1941,12 -1927,12 +1932,12 @@@ static void emit_other(struct scoreboar
        int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
  
        get_commit_info(suspect->commit, &ci, 1);
 -      sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
 +      oid_to_hex_r(hex, &suspect->commit->object.oid);
  
        cp = nth_line(sb, ent->lno);
        for (cnt = 0; cnt < ent->num_lines; cnt++) {
                char ch;
 -              int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? 40 : abbrev;
 +              int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
  
                if (suspect->commit->object.flags & UNINTERESTING) {
                        if (blank_boundary)
@@@ -2225,8 -2211,6 +2216,8 @@@ static int git_blame_config(const char 
                return 0;
        }
  
 +      if (git_diff_heuristic_config(var, value, cb) < 0)
 +              return -1;
        if (userdiff_config(var, value) < 0)
                return -1;
  
@@@ -2239,12 -2223,12 +2230,12 @@@ static void verify_working_tree_path(st
        int pos;
  
        for (parents = work_tree->parents; parents; parents = parents->next) {
 -              const unsigned char *commit_sha1 = parents->item->object.oid.hash;
 -              unsigned char blob_sha1[20];
 +              const struct object_id *commit_oid = &parents->item->object.oid;
 +              struct object_id blob_oid;
                unsigned mode;
  
 -              if (!get_tree_entry(commit_sha1, path, blob_sha1, &mode) &&
 -                  sha1_object_info(blob_sha1, NULL) == OBJ_BLOB)
 +              if (!get_tree_entry(commit_oid->hash, path, blob_oid.hash, &mode) &&
 +                  sha1_object_info(blob_oid.hash, NULL) == OBJ_BLOB)
                        return;
        }
  
                die("no such path '%s' in HEAD", path);
  }
  
 -static struct commit_list **append_parent(struct commit_list **tail, const unsigned char *sha1)
 +static struct commit_list **append_parent(struct commit_list **tail, const struct object_id *oid)
  {
        struct commit *parent;
  
 -      parent = lookup_commit_reference(sha1);
 +      parent = lookup_commit_reference(oid->hash);
        if (!parent)
 -              die("no such commit %s", sha1_to_hex(sha1));
 +              die("no such commit %s", oid_to_hex(oid));
        return &commit_list_insert(parent, tail)->next;
  }
  
@@@ -2281,10 -2265,10 +2272,10 @@@ static void append_merge_parents(struc
        }
  
        while (!strbuf_getwholeline_fd(&line, merge_head, '\n')) {
 -              unsigned char sha1[20];
 -              if (line.len < 40 || get_sha1_hex(line.buf, sha1))
 +              struct object_id oid;
 +              if (line.len < GIT_SHA1_HEXSZ || get_oid_hex(line.buf, &oid))
                        die("unknown line in '%s': %s", git_path_merge_head(), line.buf);
 -              tail = append_parent(tail, sha1);
 +              tail = append_parent(tail, &oid);
        }
        close(merge_head);
        strbuf_release(&line);
@@@ -2313,7 -2297,7 +2304,7 @@@ static struct commit *fake_working_tree
        struct commit *commit;
        struct origin *origin;
        struct commit_list **parent_tail, *parent;
 -      unsigned char head_sha1[20];
 +      struct object_id head_oid;
        struct strbuf buf = STRBUF_INIT;
        const char *ident;
        time_t now;
        commit->date = now;
        parent_tail = &commit->parents;
  
 -      if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL))
 +      if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_oid.hash, NULL))
                die("no such ref: HEAD");
  
 -      parent_tail = append_parent(parent_tail, head_sha1);
 +      parent_tail = append_parent(parent_tail, &head_oid);
        append_merge_parents(parent_tail);
        verify_working_tree_path(commit, path);
  
                switch (st.st_mode & S_IFMT) {
                case S_IFREG:
                        if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
 -                          textconv_object(read_from, mode, null_sha1, 0, &buf_ptr, &buf_len))
 +                          textconv_object(read_from, mode, &null_oid, 0, &buf_ptr, &buf_len))
                                strbuf_attach(&buf, buf_ptr, buf_len, buf_len + 1);
                        else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)
                                die_errno("cannot open or read '%s'", read_from);
        convert_to_git(path, buf.buf, buf.len, &buf, 0);
        origin->file.ptr = buf.buf;
        origin->file.size = buf.len;
 -      pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);
 +      pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_oid.hash);
  
        /*
         * Read the current index, replace the path entry with
        }
        size = cache_entry_size(len);
        ce = xcalloc(1, size);
 -      hashcpy(ce->sha1, origin->blob_sha1);
 +      oidcpy(&ce->oid, &origin->blob_oid);
        memcpy(ce->name, path, len);
        ce->ce_flags = create_ce_flags(0);
        ce->ce_namelen = len;
@@@ -2461,41 -2445,6 +2452,41 @@@ static char *prepare_final(struct score
        return xstrdup_or_null(name);
  }
  
 +static const char *dwim_reverse_initial(struct scoreboard *sb)
 +{
 +      /*
 +       * DWIM "git blame --reverse ONE -- PATH" as
 +       * "git blame --reverse ONE..HEAD -- PATH" but only do so
 +       * when it makes sense.
 +       */
 +      struct object *obj;
 +      struct commit *head_commit;
 +      unsigned char head_sha1[20];
 +
 +      if (sb->revs->pending.nr != 1)
 +              return NULL;
 +
 +      /* Is that sole rev a committish? */
 +      obj = sb->revs->pending.objects[0].item;
 +      obj = deref_tag(obj, NULL, 0);
 +      if (obj->type != OBJ_COMMIT)
 +              return NULL;
 +
 +      /* Do we have HEAD? */
 +      if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL))
 +              return NULL;
 +      head_commit = lookup_commit_reference_gently(head_sha1, 1);
 +      if (!head_commit)
 +              return NULL;
 +
 +      /* Turn "ONE" into "ONE..HEAD" then */
 +      obj->flags |= UNINTERESTING;
 +      add_pending_object(sb->revs, &head_commit->object, "HEAD");
 +
 +      sb->final = (struct commit *)obj;
 +      return sb->revs->pending.objects[0].name;
 +}
 +
  static char *prepare_initial(struct scoreboard *sb)
  {
        int i;
                if (obj->type != OBJ_COMMIT)
                        die("Non commit %s?", revs->pending.objects[i].name);
                if (sb->final)
 -                      die("More than one commit to dig down to %s and %s?",
 +                      die("More than one commit to dig up from, %s and %s?",
                            revs->pending.objects[i].name,
                            final_commit_name);
                sb->final = (struct commit *) obj;
                final_commit_name = revs->pending.objects[i].name;
        }
 +
        if (!final_commit_name)
 -              die("No commit to dig down to?");
 +              final_commit_name = dwim_reverse_initial(sb);
 +      if (!final_commit_name)
 +              die("No commit to dig up from?");
        return xstrdup(final_commit_name);
  }
  
@@@ -2595,14 -2541,6 +2586,14 @@@ int cmd_blame(int argc, const char **ar
                OPT_BIT('s', NULL, &output_option, N_("Suppress author name and timestamp (Default: off)"), OUTPUT_NO_AUTHOR),
                OPT_BIT('e', "show-email", &output_option, N_("Show author email instead of name (Default: off)"), OUTPUT_SHOW_EMAIL),
                OPT_BIT('w', NULL, &xdl_opts, N_("Ignore whitespace differences"), XDF_IGNORE_WHITESPACE),
 +
 +              /*
 +               * The following two options are parsed by parse_revision_opt()
 +               * and are only included here to get included in the "-h"
 +               * output:
 +               */
 +              { OPTION_LOWLEVEL_CALLBACK, 0, "indent-heuristic", NULL, NULL, N_("Use an experimental heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
 +
                OPT_BIT(0, "minimal", &xdl_opts, N_("Spend extra cycles to find better match"), XDF_NEED_MINIMAL),
                OPT_STRING('S', NULL, &revs_file, N_("file"), N_("Use revisions from <file> instead of calling git-rev-list")),
                OPT_STRING(0, "contents", &contents_from, N_("file"), N_("Use <file>'s contents as the final image")),
        }
  parse_done:
        no_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);
 +      xdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC;
        DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
        argc = parse_options_end(&ctx);
  
        if (incremental || (output_option & OUTPUT_PORCELAIN)) {
                if (show_progress > 0)
 -                      die("--progress can't be used with --incremental or porcelain formats");
 +                      die(_("--progress can't be used with --incremental or porcelain formats"));
                show_progress = 0;
        } else if (show_progress < 0)
                show_progress = isatty(2);
  
 -      if (0 < abbrev)
 +      if (0 < abbrev && abbrev < GIT_SHA1_HEXSZ)
                /* one more abbrev length is needed for the boundary commit */
                abbrev++;
 +      else if (!abbrev)
 +              abbrev = GIT_SHA1_HEXSZ;
  
        if (revs_file && read_ancestry(revs_file))
                die_errno("reading graft file '%s' failed", revs_file);
                sb.commits.compare = compare_commits_by_commit_date;
        }
        else if (contents_from)
 -              die("--contents and --reverse do not blend well.");
 +              die(_("--contents and --reverse do not blend well."));
        else {
                final_commit_name = prepare_initial(&sb);
                sb.commits.compare = compare_commits_by_reverse_commit_date;
                add_pending_object(&revs, &(sb.final->object), ":");
        }
        else if (contents_from)
 -              die("Cannot use --contents with final commit object name");
 +              die(_("cannot use --contents with final commit object name"));
  
        if (reverse && revs.first_parent_only) {
                final_commit = find_single_final(sb.revs, NULL);
                if (!final_commit)
 -                      die("--reverse and --first-parent together require specified latest commit");
 +                      die(_("--reverse and --first-parent together require specified latest commit"));
        }
  
        /*
                }
  
                if (oidcmp(&c->object.oid, &sb.final->object.oid))
 -                      die("--reverse --first-parent together require range along first-parent chain");
 +                      die(_("--reverse --first-parent together require range along first-parent chain"));
        }
  
        if (is_null_oid(&sb.final->object.oid)) {
        else {
                o = get_origin(&sb, sb.final, path);
                if (fill_blob_sha1_and_mode(o))
 -                      die("no such path %s in %s", path, final_commit_name);
 +                      die(_("no such path %s in %s"), path, final_commit_name);
  
                if (DIFF_OPT_TST(&sb.revs->diffopt, ALLOW_TEXTCONV) &&
 -                  textconv_object(path, o->mode, o->blob_sha1, 1, (char **) &sb.final_buf,
 +                  textconv_object(path, o->mode, &o->blob_oid, 1, (char **) &sb.final_buf,
                                    &sb.final_buf_size))
                        ;
                else
 -                      sb.final_buf = read_sha1_file(o->blob_sha1, &type,
 +                      sb.final_buf = read_sha1_file(o->blob_oid.hash, &type,
                                                      &sb.final_buf_size);
  
                if (!sb.final_buf)
 -                      die("Cannot read blob %s for path %s",
 -                          sha1_to_hex(o->blob_sha1),
 +                      die(_("cannot read blob %s for path %s"),
 +                          oid_to_hex(&o->blob_oid),
                            path);
        }
        num_read_blob++;
                                    &bottom, &top, sb.path))
                        usage(blame_usage);
                if (lno < top || ((lno || bottom) && lno < bottom))
 -                      die("file %s has only %lu lines", path, lno);
 +                      die(Q_("file %s has only %lu line",
 +                             "file %s has only %lu lines",
 +                             lno), path, lno);
                if (bottom < 1)
                        bottom = 1;
                if (top < 1)