Merge branch 'jc/diff-filter-negation'
authorJunio C Hamano <gitster@pobox.com>
Mon, 9 Sep 2013 21:28:35 +0000 (14:28 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 9 Sep 2013 21:28:35 +0000 (14:28 -0700)
Teach "git diff --diff-filter" to express "I do not want to see
these classes of changes" more directly by listing only the
unwanted ones in lowercase (e.g. "--diff-filter=d" will show
everything but deletion) and deprecate "diff-files -q" which did
the same thing as "--diff-filter=d".

* jc/diff-filter-negation:
diff: deprecate -q option to diff-files
diff: allow lowercase letter to specify what change class to exclude
diff: reject unknown change class given to --diff-filter
diff: preparse --diff-filter string argument
diff: factor out match_filter()
diff: pass the whole diff_options to diffcore_apply_filter()

1  2 
diff-lib.c
diff-no-index.c
diff.c
diff --combined diff-lib.c
index b6f4b21637fe66a614a0dd6581f90fc12e80a6f8,4634b291993ce77c1d9c587cb535e8cc6b4f076f..f1a9053168dbf0f3b5db2b2e51525828408f88ac
@@@ -64,9 -64,8 +64,9 @@@ static int check_removed(const struct c
   * commits, untracked content and/or modified content).
   */
  static int match_stat_with_submodule(struct diff_options *diffopt,
 -                                    struct cache_entry *ce, struct stat *st,
 -                                    unsigned ce_option, unsigned *dirty_submodule)
 +                                   const struct cache_entry *ce,
 +                                   struct stat *st, unsigned ce_option,
 +                                   unsigned *dirty_submodule)
  {
        int changed = ce_match_stat(ce, st, ce_option);
        if (S_ISGITLINK(ce->ce_mode)) {
@@@ -87,10 -86,12 +87,12 @@@ int run_diff_files(struct rev_info *rev
  {
        int entries, i;
        int diff_unmerged_stage = revs->max_count;
-       int silent_on_removed = option & DIFF_SILENT_ON_REMOVED;
        unsigned ce_option = ((option & DIFF_RACY_IS_MODIFIED)
                              ? CE_MATCH_RACY_IS_DIRTY : 0);
  
+       if (option & DIFF_SILENT_ON_REMOVED)
+               handle_deprecated_show_diff_q(&revs->diffopt);
        diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/");
  
        if (diff_unmerged_stage < 0)
                                        perror(ce->name);
                                        continue;
                                }
-                               if (silent_on_removed)
-                                       continue;
                                wt_mode = 0;
                        }
                        dpath->mode = wt_mode;
                                perror(ce->name);
                                continue;
                        }
-                       if (silent_on_removed)
-                               continue;
                        diff_addremove(&revs->diffopt, '-', ce->ce_mode,
                                       ce->sha1, !is_null_sha1(ce->sha1),
                                       ce->name, 0);
  /* A file entry went away or appeared */
  static void diff_index_show_file(struct rev_info *revs,
                                 const char *prefix,
 -                               struct cache_entry *ce,
 +                               const struct cache_entry *ce,
                                 const unsigned char *sha1, int sha1_valid,
                                 unsigned int mode,
                                 unsigned dirty_submodule)
                       sha1, sha1_valid, ce->name, dirty_submodule);
  }
  
 -static int get_stat_data(struct cache_entry *ce,
 +static int get_stat_data(const struct cache_entry *ce,
                         const unsigned char **sha1p,
                         unsigned int *modep,
                         int cached, int match_missing,
  }
  
  static void show_new_file(struct rev_info *revs,
 -                        struct cache_entry *new,
 +                        const struct cache_entry *new,
                          int cached, int match_missing)
  {
        const unsigned char *sha1;
  }
  
  static int show_modified(struct rev_info *revs,
 -                       struct cache_entry *old,
 -                       struct cache_entry *new,
 +                       const struct cache_entry *old,
 +                       const struct cache_entry *new,
                         int report_missing,
                         int cached, int match_missing)
  {
   * give you the position and number of entries in the index).
   */
  static void do_oneway_diff(struct unpack_trees_options *o,
 -      struct cache_entry *idx,
 -      struct cache_entry *tree)
 +                         const struct cache_entry *idx,
 +                         const struct cache_entry *tree)
  {
        struct rev_info *revs = o->unpack_data;
        int match_missing, cached;
   * the fairly complex unpack_trees() semantic requirements, including
   * the skipping, the path matching, the type conflict cases etc.
   */
 -static int oneway_diff(struct cache_entry **src, struct unpack_trees_options *o)
 +static int oneway_diff(const struct cache_entry * const *src,
 +                     struct unpack_trees_options *o)
  {
 -      struct cache_entry *idx = src[0];
 -      struct cache_entry *tree = src[1];
 +      const struct cache_entry *idx = src[0];
 +      const struct cache_entry *tree = src[1];
        struct rev_info *revs = o->unpack_data;
  
        /*
diff --combined diff-no-index.c
index e66fdf33da9e3dd3da561aadffec938e6bc2ad49,a788a5f3043400e6adee5436ca5618690b689de2..e301aafff9ff230313425eea5987b18056b72c98
@@@ -45,7 -45,7 +45,7 @@@ static int get_mode(const char *path, i
  
        if (!path || !strcmp(path, "/dev/null"))
                *mode = 0;
 -#ifdef _WIN32
 +#ifdef GIT_WINDOWS_NATIVE
        else if (!strcasecmp(path, "nul"))
                *mode = 0;
  #endif
@@@ -187,7 -187,7 +187,7 @@@ void diff_no_index(struct rev_info *rev
  {
        int i, prefixlen;
        int no_index = 0;
-       unsigned options = 0;
+       unsigned deprecated_show_diff_q_option_used = 0;
        const char *paths[2];
  
        /* Were we asked to do --no-index explicitly? */
                if (!strcmp(argv[i], "--no-index"))
                        i++;
                else if (!strcmp(argv[i], "-q")) {
-                       options |= DIFF_SILENT_ON_REMOVED;
+                       deprecated_show_diff_q_option_used = 1;
                        i++;
                }
                else if (!strcmp(argv[i], "--"))
        revs->max_count = -2;
        diff_setup_done(&revs->diffopt);
  
+       if (deprecated_show_diff_q_option_used)
+               handle_deprecated_show_diff_q(&revs->diffopt);
        setup_diff_pager(&revs->diffopt);
        DIFF_OPT_SET(&revs->diffopt, EXIT_WITH_STATUS);
  
diff --combined diff.c
index 061694b5ea9f860a03d569b7a6bc438b9bcbf8a7,78819ba1513acbd726076873bab92a5183f2827b..a04a34d0487f231481a7c9c475ac4a3e92b5ed2d
--- 1/diff.c
--- 2/diff.c
+++ b/diff.c
@@@ -669,7 -669,7 +669,7 @@@ static void emit_rewrite_diff(const cha
        memset(&ecbdata, 0, sizeof(ecbdata));
        ecbdata.color_diff = want_color(o->use_color);
        ecbdata.found_changesp = &o->found_changes;
 -      ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
 +      ecbdata.ws_rule = whitespace_rule(name_b);
        ecbdata.opt = o;
        if (ecbdata.ws_rule & WS_BLANK_AT_EOF) {
                mmfile_t mf1, mf2;
@@@ -1683,7 -1683,9 +1683,7 @@@ static void show_stats(struct diffstat_
                del = deleted;
  
                if (graph_width <= max_change) {
 -                      int total = add + del;
 -
 -                      total = scale_linear(add + del, graph_width, max_change);
 +                      int total = scale_linear(add + del, graph_width, max_change);
                        if (total < 2 && add && del)
                                /* width >= 2 due to the sanity check */
                                total = 2;
@@@ -2252,8 -2254,7 +2252,8 @@@ static void builtin_diff(const char *na
                        (!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 ? one->path : two->path,
 +              show_submodule_summary(o->file, one->path ? one->path : two->path,
 +                              line_prefix,
                                one->sha1, two->sha1, two->dirty_submodule,
                                meta, del, add, reset);
                return;
                ecbdata.label_path = lbl;
                ecbdata.color_diff = want_color(o->use_color);
                ecbdata.found_changesp = &o->found_changes;
 -              ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
 +              ecbdata.ws_rule = whitespace_rule(name_b);
                if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
                        check_blank_at_eof(&mf1, &mf2, &ecbdata);
                ecbdata.opt = o;
@@@ -2584,7 -2585,7 +2584,7 @@@ void fill_filespec(struct diff_filespe
   */
  static int reuse_worktree_file(const char *name, const unsigned char *sha1, int want_file)
  {
 -      struct cache_entry *ce;
 +      const struct cache_entry *ce;
        struct stat st;
        int pos, len;
  
@@@ -2675,14 -2676,6 +2675,14 @@@ static int diff_populate_gitlink(struc
  int diff_populate_filespec(struct diff_filespec *s, int size_only)
  {
        int err = 0;
 +      /*
 +       * demote FAIL to WARN to allow inspecting the situation
 +       * instead of refusing.
 +       */
 +      enum safe_crlf crlf_warn = (safe_crlf == SAFE_CRLF_FAIL
 +                                  ? SAFE_CRLF_WARN
 +                                  : safe_crlf);
 +
        if (!DIFF_FILE_VALID(s))
                die("internal error: asking to populate invalid file.");
        if (S_ISDIR(s->mode))
                /*
                 * Convert from working tree format to canonical git format
                 */
 -              if (convert_to_git(s->path, s->data, s->size, &buf, safe_crlf)) {
 +              if (convert_to_git(s->path, s->data, s->size, &buf, crlf_warn)) {
                        size_t size = 0;
                        munmap(s->data, s->size);
                        s->should_munmap = 0;
@@@ -3503,11 -3496,88 +3503,93 @@@ static int parse_submodule_opt(struct d
        return 1;
  }
  
+ static const char diff_status_letters[] = {
+       DIFF_STATUS_ADDED,
+       DIFF_STATUS_COPIED,
+       DIFF_STATUS_DELETED,
+       DIFF_STATUS_MODIFIED,
+       DIFF_STATUS_RENAMED,
+       DIFF_STATUS_TYPE_CHANGED,
+       DIFF_STATUS_UNKNOWN,
+       DIFF_STATUS_UNMERGED,
+       DIFF_STATUS_FILTER_AON,
+       DIFF_STATUS_FILTER_BROKEN,
+       '\0',
+ };
+ static unsigned int filter_bit['Z' + 1];
+ static void prepare_filter_bits(void)
+ {
+       int i;
+       if (!filter_bit[DIFF_STATUS_ADDED]) {
+               for (i = 0; diff_status_letters[i]; i++)
+                       filter_bit[(int) diff_status_letters[i]] = (1 << i);
+       }
+ }
+ static unsigned filter_bit_tst(char status, const struct diff_options *opt)
+ {
+       return opt->filter & filter_bit[(int) status];
+ }
+ static int parse_diff_filter_opt(const char *optarg, struct diff_options *opt)
+ {
+       int i, optch;
+       prepare_filter_bits();
+       /*
+        * If there is a negation e.g. 'd' in the input, and we haven't
+        * initialized the filter field with another --diff-filter, start
+        * from full set of bits, except for AON.
+        */
+       if (!opt->filter) {
+               for (i = 0; (optch = optarg[i]) != '\0'; i++) {
+                       if (optch < 'a' || 'z' < optch)
+                               continue;
+                       opt->filter = (1 << (ARRAY_SIZE(diff_status_letters) - 1)) - 1;
+                       opt->filter &= ~filter_bit[DIFF_STATUS_FILTER_AON];
+                       break;
+               }
+       }
+       for (i = 0; (optch = optarg[i]) != '\0'; i++) {
+               unsigned int bit;
+               int negate;
+               if ('a' <= optch && optch <= 'z') {
+                       negate = 1;
+                       optch = toupper(optch);
+               } else {
+                       negate = 0;
+               }
+               bit = (0 <= optch && optch <= 'Z') ? filter_bit[optch] : 0;
+               if (!bit)
+                       return optarg[i];
+               if (negate)
+                       opt->filter &= ~bit;
+               else
+                       opt->filter |= bit;
+       }
+       return 0;
+ }
+ /* Used only by "diff-files" and "diff --no-index" */
+ void handle_deprecated_show_diff_q(struct diff_options *opt)
+ {
+       warning("'diff -q' and 'diff-files -q' are deprecated.");
+       warning("Use 'diff --diff-filter=d' instead to ignore deleted filepairs.");
+       parse_diff_filter_opt("d", opt);
+ }
 +static void enable_patch_output(int *fmt) {
 +      *fmt &= ~DIFF_FORMAT_NO_OUTPUT;
 +      *fmt |= DIFF_FORMAT_PATCH;
 +}
 +
  int diff_opt_parse(struct diff_options *options, const char **av, int ac)
  {
        const char *arg = av[0];
        int argcount;
  
        /* Output format options */
 -      if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch"))
 -              options->output_format |= DIFF_FORMAT_PATCH;
 -      else if (opt_arg(arg, 'U', "unified", &options->context))
 -              options->output_format |= DIFF_FORMAT_PATCH;
 +      if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch")
 +          || opt_arg(arg, 'U', "unified", &options->context))
 +              enable_patch_output(&options->output_format);
        else if (!strcmp(arg, "--raw"))
                options->output_format |= DIFF_FORMAT_RAW;
 -      else if (!strcmp(arg, "--patch-with-raw"))
 -              options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW;
 -      else if (!strcmp(arg, "--numstat"))
 +      else if (!strcmp(arg, "--patch-with-raw")) {
 +              enable_patch_output(&options->output_format);
 +              options->output_format |= DIFF_FORMAT_RAW;
 +      } else if (!strcmp(arg, "--numstat"))
                options->output_format |= DIFF_FORMAT_NUMSTAT;
        else if (!strcmp(arg, "--shortstat"))
                options->output_format |= DIFF_FORMAT_SHORTSTAT;
                options->output_format |= DIFF_FORMAT_CHECKDIFF;
        else if (!strcmp(arg, "--summary"))
                options->output_format |= DIFF_FORMAT_SUMMARY;
 -      else if (!strcmp(arg, "--patch-with-stat"))
 -              options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
 -      else if (!strcmp(arg, "--name-only"))
 +      else if (!strcmp(arg, "--patch-with-stat")) {
 +              enable_patch_output(&options->output_format);
 +              options->output_format |= DIFF_FORMAT_DIFFSTAT;
 +      } else if (!strcmp(arg, "--name-only"))
                options->output_format |= DIFF_FORMAT_NAME;
        else if (!strcmp(arg, "--name-status"))
                options->output_format |= DIFF_FORMAT_NAME_STATUS;
 -      else if (!strcmp(arg, "-s"))
 +      else if (!strcmp(arg, "-s") || !strcmp(arg, "--no-patch"))
                options->output_format |= DIFF_FORMAT_NO_OUTPUT;
        else if (!prefixcmp(arg, "--stat"))
                /* --stat, --stat-width, --stat-name-width, or --stat-count */
                DIFF_XDL_SET(options, IGNORE_WHITESPACE_CHANGE);
        else if (!strcmp(arg, "--ignore-space-at-eol"))
                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, "--patience"))
                options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
        else if (!strcmp(arg, "--histogram"))
  
        /* flags options */
        else if (!strcmp(arg, "--binary")) {
 -              options->output_format |= DIFF_FORMAT_PATCH;
 +              enable_patch_output(&options->output_format);
                DIFF_OPT_SET(options, BINARY);
        }
        else if (!strcmp(arg, "--full-index"))
                return argcount;
        }
        else if ((argcount = parse_long_opt("diff-filter", av, &optarg))) {
-               options->filter = optarg;
+               int offending = parse_diff_filter_opt(optarg, options);
+               if (offending)
+                       die("unknown change class '%c' in --diff-filter=%s",
+                           offending, optarg);
                return argcount;
        }
        else if (!strcmp(arg, "--abbrev"))
@@@ -4464,7 -4534,7 +4549,7 @@@ void diff_flush(struct diff_options *op
            DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
                /*
                 * run diff_flush_patch for the exit status. setting
 -               * options->file to /dev/null should be safe, becaue we
 +               * options->file to /dev/null should be safe, because we
                 * aren't supposed to produce any output anyway.
                 */
                if (options->close_file)
@@@ -4524,27 -4594,32 +4609,32 @@@ free_queue
        }
  }
  
- static void diffcore_apply_filter(const char *filter)
+ static int match_filter(const struct diff_options *options, const struct diff_filepair *p)
+ {
+       return (((p->status == DIFF_STATUS_MODIFIED) &&
+                ((p->score &&
+                  filter_bit_tst(DIFF_STATUS_FILTER_BROKEN, options)) ||
+                 (!p->score &&
+                  filter_bit_tst(DIFF_STATUS_MODIFIED, options)))) ||
+               ((p->status != DIFF_STATUS_MODIFIED) &&
+                filter_bit_tst(p->status, options)));
+ }
+ static void diffcore_apply_filter(struct diff_options *options)
  {
        int i;
        struct diff_queue_struct *q = &diff_queued_diff;
        struct diff_queue_struct outq;
        DIFF_QUEUE_CLEAR(&outq);
  
-       if (!filter)
+       if (!options->filter)
                return;
  
-       if (strchr(filter, DIFF_STATUS_FILTER_AON)) {
+       if (filter_bit_tst(DIFF_STATUS_FILTER_AON, options)) {
                int found;
                for (i = found = 0; !found && i < q->nr; i++) {
-                       struct diff_filepair *p = q->queue[i];
-                       if (((p->status == DIFF_STATUS_MODIFIED) &&
-                            ((p->score &&
-                              strchr(filter, DIFF_STATUS_FILTER_BROKEN)) ||
-                             (!p->score &&
-                              strchr(filter, DIFF_STATUS_MODIFIED)))) ||
-                           ((p->status != DIFF_STATUS_MODIFIED) &&
-                            strchr(filter, p->status)))
+                       if (match_filter(options, q->queue[i]))
                                found++;
                }
                if (found)
                /* Only the matching ones */
                for (i = 0; i < q->nr; i++) {
                        struct diff_filepair *p = q->queue[i];
-                       if (((p->status == DIFF_STATUS_MODIFIED) &&
-                            ((p->score &&
-                              strchr(filter, DIFF_STATUS_FILTER_BROKEN)) ||
-                             (!p->score &&
-                              strchr(filter, DIFF_STATUS_MODIFIED)))) ||
-                           ((p->status != DIFF_STATUS_MODIFIED) &&
-                            strchr(filter, p->status)))
+                       if (match_filter(options, p))
                                diff_q(&outq, p);
                        else
                                diff_free_filepair(p);
@@@ -4676,7 -4744,7 +4759,7 @@@ void diffcore_std(struct diff_options *
        if (!options->found_follow)
                /* See try_to_follow_renames() in tree-diff.c */
                diff_resolve_rename_copy();
-       diffcore_apply_filter(options->filter);
+       diffcore_apply_filter(options);
  
        if (diff_queued_diff.nr && !DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
                DIFF_OPT_SET(options, HAS_CHANGES);