From: Junio C Hamano Date: Wed, 26 Oct 2016 20:14:42 +0000 (-0700) Subject: Merge branch 'jc/diff-unique-abbrev-comments' X-Git-Tag: v2.11.0-rc0~50 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/c334effa2332e30199451695b148f02eb7d9fd13?hp=-c Merge branch 'jc/diff-unique-abbrev-comments' A bit more comments in a tricky code. * jc/diff-unique-abbrev-comments: diff_unique_abbrev(): document its assumption and limitation --- c334effa2332e30199451695b148f02eb7d9fd13 diff --combined diff.c index 1d304e0550,01e1507e74..f5d6d7e4f6 --- a/diff.c +++ b/diff.c @@@ -18,7 -18,6 +18,7 @@@ #include "ll-merge.h" #include "string-list.h" #include "argv-array.h" +#include "graph.h" #ifdef NO_FAST_WORKING_DIRECTORY #define FAST_WORKING_DIRECTORY 0 @@@ -27,7 -26,6 +27,7 @@@ #endif static int diff_detect_rename_default; +static int diff_indent_heuristic; /* experimental */ static int diff_compaction_heuristic; /* experimental */ static int diff_rename_limit_default = 400; static int diff_suppress_blank_empty; @@@ -56,11 -54,6 +56,11 @@@ static char diff_colors[][COLOR_MAXLEN GIT_COLOR_NORMAL, /* FUNCINFO */ }; +static NORETURN void die_want_option(const char *option_name) +{ + die(_("option '%s' requires a value"), option_name); +} + static int parse_diff_color_slot(const char *var) { if (!strcasecmp(var, "context") || !strcasecmp(var, "plain")) @@@ -138,11 -131,9 +138,11 @@@ static int parse_dirstat_params(struct static int parse_submodule_params(struct diff_options *options, const char *value) { if (!strcmp(value, "log")) - DIFF_OPT_SET(options, SUBMODULE_LOG); + options->submodule_format = DIFF_SUBMODULE_LOG; else if (!strcmp(value, "short")) - DIFF_OPT_CLR(options, SUBMODULE_LOG); + options->submodule_format = DIFF_SUBMODULE_SHORT; + else if (!strcmp(value, "diff")) + options->submodule_format = DIFF_SUBMODULE_INLINE_DIFF; else return -1; return 0; @@@ -183,21 -174,6 +183,21 @@@ void init_diff_ui_defaults(void diff_detect_rename_default = 1; } +int git_diff_heuristic_config(const char *var, const char *value, void *cb) +{ + if (!strcmp(var, "diff.indentheuristic")) { + diff_indent_heuristic = git_config_bool(var, value); + if (diff_indent_heuristic) + diff_compaction_heuristic = 0; + } + if (!strcmp(var, "diff.compactionheuristic")) { + diff_compaction_heuristic = git_config_bool(var, value); + if (diff_compaction_heuristic) + diff_indent_heuristic = 0; + } + return 0; +} + int git_diff_ui_config(const char *var, const char *value, void *cb) { if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) { @@@ -214,6 -190,10 +214,6 @@@ diff_detect_rename_default = git_config_rename(var, value); return 0; } - if (!strcmp(var, "diff.compactionheuristic")) { - diff_compaction_heuristic = git_config_bool(var, value); - return 0; - } if (!strcmp(var, "diff.autorefreshindex")) { diff_auto_refresh_index = git_config_bool(var, value); return 0; @@@ -254,8 -234,6 +254,8 @@@ return 0; } + if (git_diff_heuristic_config(var, value, cb) < 0) + return -1; if (git_color_config(var, value, cb) < 0) return -1; @@@ -1638,7 -1616,7 +1638,7 @@@ static void show_stats(struct diffstat_ */ if (options->stat_width == -1) - width = term_columns() - options->output_prefix_length; + width = term_columns() - strlen(line_prefix); else width = options->stat_width ? options->stat_width : 80; number_width = decimal_width(max_change) > number_width ? @@@ -2019,7 -1997,7 +2019,7 @@@ found_damage return; /* Show all directories with more than x% of the changes */ - qsort(dir.files, dir.nr, sizeof(dir.files[0]), dirstat_compare); + QSORT(dir.files, dir.nr, dirstat_compare); gather_dirstat(options, &dir, changed, "", 0); } @@@ -2063,7 -2041,7 +2063,7 @@@ static void show_dirstat_by_line(struc return; /* Show all directories with more than x% of the changes */ - qsort(dir.files, dir.nr, sizeof(dir.files[0]), dirstat_compare); + QSORT(dir.files, dir.nr, dirstat_compare); gather_dirstat(options, &dir, changed, "", 0); } @@@ -2312,37 -2290,17 +2312,37 @@@ static void builtin_diff(const char *na struct strbuf header = STRBUF_INIT; const char *line_prefix = diff_line_prefix(o); - if (DIFF_OPT_TST(o, SUBMODULE_LOG) && - (!one->mode || S_ISGITLINK(one->mode)) && - (!two->mode || S_ISGITLINK(two->mode))) { + diff_set_mnemonic_prefix(o, "a/", "b/"); + if (DIFF_OPT_TST(o, REVERSE_DIFF)) { + a_prefix = o->b_prefix; + b_prefix = o->a_prefix; + } else { + a_prefix = o->a_prefix; + b_prefix = o->b_prefix; + } + + if (o->submodule_format == DIFF_SUBMODULE_LOG && + (!one->mode || S_ISGITLINK(one->mode)) && + (!two->mode || S_ISGITLINK(two->mode))) { const char *del = diff_get_color_opt(o, DIFF_FILE_OLD); const char *add = diff_get_color_opt(o, DIFF_FILE_NEW); show_submodule_summary(o->file, one->path ? one->path : two->path, line_prefix, - one->oid.hash, two->oid.hash, + &one->oid, &two->oid, two->dirty_submodule, meta, del, add, reset); return; + } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF && + (!one->mode || S_ISGITLINK(one->mode)) && + (!two->mode || S_ISGITLINK(two->mode))) { + const char *del = diff_get_color_opt(o, DIFF_FILE_OLD); + const char *add = diff_get_color_opt(o, DIFF_FILE_NEW); + show_submodule_inline_diff(o->file, one->path ? one->path : two->path, + line_prefix, + &one->oid, &two->oid, + two->dirty_submodule, + meta, del, add, reset, o); + return; } if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) { @@@ -2350,6 -2308,15 +2350,6 @@@ textconv_two = get_textconv(two); } - diff_set_mnemonic_prefix(o, "a/", "b/"); - if (DIFF_OPT_TST(o, REVERSE_DIFF)) { - a_prefix = o->b_prefix; - b_prefix = o->a_prefix; - } else { - a_prefix = o->a_prefix; - b_prefix = o->b_prefix; - } - /* Never use a non-valid filename anywhere if at all possible */ name_a = DIFF_FILE_VALID(one) ? name_a : name_b; name_b = DIFF_FILE_VALID(two) ? name_b : name_a; @@@ -2723,7 -2690,7 +2723,7 @@@ static int reuse_worktree_file(const ch * This is not the sha1 we are looking for, or * unreusable because it is not a regular file. */ - if (hashcmp(sha1, ce->sha1) || !S_ISREG(ce->ce_mode)) + if (hashcmp(sha1, ce->oid.hash) || !S_ISREG(ce->ce_mode)) return 0; /* @@@ -3109,7 -3076,7 +3109,7 @@@ static void fill_metainfo(struct strbu } strbuf_addf(msg, "%s%sindex %s..", line_prefix, set, find_unique_abbrev(one->oid.hash, abbrev)); - strbuf_addstr(msg, find_unique_abbrev(two->oid.hash, abbrev)); + strbuf_add_unique_abbrev(msg, two->oid.hash, abbrev); if (one->mode == two->mode) strbuf_addf(msg, " %06o", one->mode); strbuf_addf(msg, "%s\n", reset); @@@ -3316,9 -3283,7 +3316,9 @@@ void diff_setup(struct diff_options *op options->use_color = diff_use_color_default; options->detect_rename = diff_detect_rename_default; options->xdl_opts |= diff_algorithm; - if (diff_compaction_heuristic) + if (diff_indent_heuristic) + DIFF_XDL_SET(options, INDENT_HEURISTIC); + else if (diff_compaction_heuristic) DIFF_XDL_SET(options, COMPACTION_HEURISTIC); options->orderfile = diff_order_file_cfg; @@@ -3347,7 -3312,7 +3347,7 @@@ void diff_setup_done(struct diff_option if (options->output_format & DIFF_FORMAT_NO_OUTPUT) count++; if (count > 1) - die("--name-only, --name-status, --check and -s are mutually exclusive"); + die(_("--name-only, --name-status, --check and -s are mutually exclusive")); /* * Most of the time we can say "there are changes" @@@ -3543,7 -3508,7 +3543,7 @@@ static int stat_opt(struct diff_option if (*arg == '=') width = strtoul(arg + 1, &end, 10); else if (!*arg && !av[1]) - die("Option '--stat-width' requires a value"); + die_want_option("--stat-width"); else if (!*arg) { width = strtoul(av[1], &end, 10); argcount = 2; @@@ -3552,7 -3517,7 +3552,7 @@@ if (*arg == '=') name_width = strtoul(arg + 1, &end, 10); else if (!*arg && !av[1]) - die("Option '--stat-name-width' requires a value"); + die_want_option("--stat-name-width"); else if (!*arg) { name_width = strtoul(av[1], &end, 10); argcount = 2; @@@ -3561,7 -3526,7 +3561,7 @@@ if (*arg == '=') graph_width = strtoul(arg + 1, &end, 10); else if (!*arg && !av[1]) - die("Option '--stat-graph-width' requires a value"); + die_want_option("--stat-graph-width"); else if (!*arg) { graph_width = strtoul(av[1], &end, 10); argcount = 2; @@@ -3570,7 -3535,7 +3570,7 @@@ if (*arg == '=') count = strtoul(arg + 1, &end, 10); else if (!*arg && !av[1]) - die("Option '--stat-count' requires a value"); + die_want_option("--stat-count"); else if (!*arg) { count = strtoul(av[1], &end, 10); argcount = 2; @@@ -3840,15 -3805,9 +3840,15 @@@ int diff_opt_parse(struct diff_options DIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL); else if (!strcmp(arg, "--ignore-blank-lines")) DIFF_XDL_SET(options, IGNORE_BLANK_LINES); - else if (!strcmp(arg, "--compaction-heuristic")) + else if (!strcmp(arg, "--indent-heuristic")) { + DIFF_XDL_SET(options, INDENT_HEURISTIC); + DIFF_XDL_CLR(options, COMPACTION_HEURISTIC); + } else if (!strcmp(arg, "--no-indent-heuristic")) + DIFF_XDL_CLR(options, INDENT_HEURISTIC); + else if (!strcmp(arg, "--compaction-heuristic")) { DIFF_XDL_SET(options, COMPACTION_HEURISTIC); - else if (!strcmp(arg, "--no-compaction-heuristic")) + DIFF_XDL_CLR(options, INDENT_HEURISTIC); + } else if (!strcmp(arg, "--no-compaction-heuristic")) DIFF_XDL_CLR(options, COMPACTION_HEURISTIC); else if (!strcmp(arg, "--patience")) options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF); @@@ -3946,7 -3905,7 +3946,7 @@@ DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG); handle_ignore_submodules_arg(options, arg); } else if (!strcmp(arg, "--submodule")) - DIFF_OPT_SET(options, SUBMODULE_LOG); + options->submodule_format = DIFF_SUBMODULE_LOG; else if (skip_prefix(arg, "--submodule=", &arg)) return parse_submodule_opt(options, arg); else if (skip_prefix(arg, "--ws-error-highlight=", &arg)) @@@ -3997,12 -3956,6 +3997,12 @@@ options->a_prefix = optarg; return argcount; } + else if ((argcount = parse_long_opt("line-prefix", av, &optarg))) { + options->line_prefix = optarg; + options->line_prefix_length = strlen(options->line_prefix); + graph_setup_line_prefix(options); + return argcount; + } else if ((argcount = parse_long_opt("dst-prefix", av, &optarg))) { options->b_prefix = optarg; return argcount; @@@ -4136,7 -4089,8 +4136,8 @@@ void diff_free_filepair(struct diff_fil free(p); } - /* This is different from find_unique_abbrev() in that + /* + * This is different from find_unique_abbrev() in that * it stuffs the result with dots for alignment. */ const char *diff_unique_abbrev(const unsigned char *sha1, int len) @@@ -4148,6 -4102,26 +4149,26 @@@ abbrev = find_unique_abbrev(sha1, len); abblen = strlen(abbrev); + + /* + * In well-behaved cases, where the abbbreviated result is the + * same as the requested length, append three dots after the + * abbreviation (hence the whole logic is limited to the case + * where abblen < 37); when the actual abbreviated result is a + * bit longer than the requested length, we reduce the number + * of dots so that they match the well-behaved ones. However, + * if the actual abbreviation is longer than the requested + * length by more than three, we give up on aligning, and add + * three dots anyway, to indicate that the output is not the + * full object name. Yes, this may be suboptimal, but this + * appears only in "diff --raw --abbrev" output and it is not + * worth the effort to change it now. Note that this would + * likely to work fine when the automatic sizing of default + * abbreviation length is used--we would be fed -1 in "len" in + * that case, and will end up always appending three-dots, but + * the automatic sizing is supposed to give abblen that ensures + * uniqueness across all objects (statistically speaking). + */ if (abblen < 37) { static char hex[41]; if (len < abblen && abblen <= len + 2) @@@ -4923,7 -4897,7 +4944,7 @@@ static int diffnamecmp(const void *a_, void diffcore_fix_diff_index(struct diff_options *options) { struct diff_queue_struct *q = &diff_queued_diff; - qsort(q->queue, q->nr, sizeof(q->queue[0]), diffnamecmp); + QSORT(q->queue, q->nr, diffnamecmp); } void diffcore_std(struct diff_options *options)