Merge branch 'nd/diff-parseopt'
authorJunio C Hamano <gitster@pobox.com>
Thu, 7 Mar 2019 00:59:52 +0000 (09:59 +0900)
committerJunio C Hamano <gitster@pobox.com>
Thu, 7 Mar 2019 00:59:52 +0000 (09:59 +0900)
The diff machinery, one of the oldest parts of the system, which
long predates the parse-options API, uses fairly long and complex
handcrafted option parser. This is being rewritten to use the
parse-options API.

* nd/diff-parseopt:
diff.c: convert --raw
diff.c: convert -W|--[no-]function-context
diff.c: convert -U|--unified
diff.c: convert -u|-p|--patch
diff.c: prepare to use parse_options() for parsing
diff.h: avoid bit fields in struct diff_flags
diff.h: keep forward struct declarations sorted
parse-options: allow ll_callback with OPTION_CALLBACK
parse-options: avoid magic return codes
parse-options: stop abusing 'callback' for lowlevel callbacks
parse-options: add OPT_BITOP()
parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN
parse-options: add one-shot mode
parse-options.h: remove extern on function prototypes

1  2 
Documentation/diff-options.txt
builtin/blame.c
builtin/merge.c
builtin/update-index.c
diff.c
index 554a34080d7081da917cd54cd34eceb7bf4cd95c,0711734b125f92d441debeea9849e0f3512790c2..f37374165d19cdf626225764e54d8b4d85c05c65
@@@ -36,7 -36,7 +36,7 @@@ endif::git-format-patch[
  -U<n>::
  --unified=<n>::
        Generate diffs with <n> lines of context instead of
-       the usual three.
+       the usual three. Implies `--patch`.
  ifndef::git-format-patch[]
        Implies `-p`.
  endif::git-format-patch[]
@@@ -293,12 -293,8 +293,12 @@@ dimmed-zebra:
        `dimmed_zebra` is a deprecated synonym.
  --
  
 +--no-color-moved::
 +      Turn off move detection. This can be used to override configuration
 +      settings. It is the same as `--color-moved=no`.
 +
  --color-moved-ws=<modes>::
 -      This configures how white spaces are ignored when performing the
 +      This configures how whitespace is ignored when performing the
        move detection for `--color-moved`.
  ifdef::git-diff[]
        It can be set by the `diff.colorMovedWS` configuration setting.
@@@ -306,8 -302,6 +306,8 @@@ endif::git-diff[
        These modes can be given as a comma separated list:
  +
  --
 +no::
 +      Do not ignore whitespace when performing move detection.
  ignore-space-at-eol::
        Ignore changes in whitespace at EOL.
  ignore-space-change::
@@@ -318,17 -312,12 +318,17 @@@ ignore-all-space:
        Ignore whitespace when comparing lines. This ignores differences
        even if one line has whitespace where the other line has none.
  allow-indentation-change::
 -      Initially ignore any white spaces in the move detection, then
 +      Initially ignore any whitespace in the move detection, then
        group the moved code blocks only into a block if the change in
        whitespace is the same per line. This is incompatible with the
        other modes.
  --
  
 +--no-color-moved-ws::
 +      Do not ignore whitespace when performing move detection. This can be
 +      used to override configuration settings. It is the same as
 +      `--color-moved-ws=no`.
 +
  --word-diff[=<mode>]::
        Show a word diff, using the <mode> to delimit changed words.
        By default, words are delimited by whitespace; see
diff --combined builtin/blame.c
index 581de0d8322681ef11026b3e36a81c7baf0fda38,8dcc55dffa9f2dd4d66ea973d3f67115fe9d7b56..177c1022a0c46dbd8983224719d4d9cc0106a9fb
@@@ -814,7 -814,7 +814,7 @@@ int cmd_blame(int argc, const char **ar
                 * 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 },
+               { OPTION_LOWLEVEL_CALLBACK, 0, "indent-heuristic", NULL, NULL, N_("Use an experimental heuristic to improve diffs"), PARSE_OPT_NOARG, NULL, 0, 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")),
@@@ -925,10 -925,6 +925,10 @@@ parse_done
                 */
                blame_date_width = utf8_strwidth(_("4 years, 11 months ago")) + 1; /* add the null */
                break;
 +      case DATE_HUMAN:
 +              /* If the year is shown, no time is shown */
 +              blame_date_width = sizeof("Thu Oct 19 16:00");
 +              break;
        case DATE_NORMAL:
                blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
                break;
                long bottom, top;
                if (parse_range_arg(range_list.items[range_i].string,
                                    nth_line_cb, &sb, lno, anchor,
 -                                  &bottom, &top, sb.path, &the_index))
 +                                  &bottom, &top, sb.path,
 +                                  the_repository->index))
                        usage(blame_usage);
                if ((!lno && (top || bottom)) || lno < bottom)
                        die(Q_("file %s has only %lu line",
diff --combined builtin/merge.c
index e47d77baeebe888cfb67f8e03804ffc3ee3715c0,563a16f38a50e13144279925442b7ab491c87f58..5ce8946d390c1c42b885a2c86a363f79aec1b27a
@@@ -6,7 -6,6 +6,7 @@@
   * Based on git-merge.sh by Junio C Hamano.
   */
  
 +#define USE_THE_INDEX_COMPATIBILITY_MACROS
  #include "cache.h"
  #include "config.h"
  #include "parse-options.h"
@@@ -113,12 -112,15 +113,15 @@@ static int option_parse_message(const s
        return 0;
  }
  
- static int option_read_message(struct parse_opt_ctx_t *ctx,
-                              const struct option *opt, int unset)
+ static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx,
+                                                const struct option *opt,
+                                                const char *arg_not_used,
+                                                int unset)
  {
        struct strbuf *buf = opt->value;
        const char *arg;
  
+       BUG_ON_OPT_ARG(arg_not_used);
        if (unset)
                BUG("-F cannot be negated");
  
@@@ -262,7 -264,7 +265,7 @@@ static struct option builtin_merge_opti
                option_parse_message),
        { OPTION_LOWLEVEL_CALLBACK, 'F', "file", &merge_msg, N_("path"),
                N_("read message from file"), PARSE_OPT_NONEG,
-               (parse_opt_cb *) option_read_message },
+               NULL, 0, option_read_message },
        OPT__VERBOSITY(&verbosity),
        OPT_BOOL(0, "abort", &abort_current_merge,
                N_("abort the current in-progress merge")),
@@@ -703,7 -705,7 +706,7 @@@ static int try_merge_strategy(const cha
                        return 2;
                }
  
 -              init_merge_options(&o);
 +              init_merge_options(&o, the_repository);
                if (!strcmp(strategy, "subtree"))
                        o.subtree_shift = "";
  
diff --combined builtin/update-index.c
index 02ace602b9ae23d6ab74abac016948410b30fcde,7abde20169ac12d88a6206d232d8cf48a1bb8a60..1b6c42f748fd52ece4cf5f109f92a50bc2a87be3
@@@ -3,7 -3,6 +3,7 @@@
   *
   * Copyright (C) Linus Torvalds, 2005
   */
 +#define USE_THE_INDEX_COMPATIBILITY_MACROS
  #include "cache.h"
  #include "config.h"
  #include "lockfile.h"
@@@ -848,14 -847,16 +848,16 @@@ static int parse_new_style_cacheinfo(co
        return 0;
  }
  
- static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
-                               const struct option *opt, int unset)
+ static enum parse_opt_result cacheinfo_callback(
+       struct parse_opt_ctx_t *ctx, const struct option *opt,
+       const char *arg, int unset)
  {
        struct object_id oid;
        unsigned int mode;
        const char *path;
  
        BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
  
        if (!parse_new_style_cacheinfo(ctx->argv[1], &mode, &oid, &path)) {
                if (add_cacheinfo(mode, &oid, path, 0))
        return 0;
  }
  
- static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
-                             const struct option *opt, int unset)
+ static enum parse_opt_result stdin_cacheinfo_callback(
+       struct parse_opt_ctx_t *ctx, const struct option *opt,
+       const char *arg, int unset)
  {
        int *nul_term_line = opt->value;
  
        BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
  
        if (ctx->argc != 1)
                return error("option '%s' must be the last argument", opt->long_name);
        return 0;
  }
  
- static int stdin_callback(struct parse_opt_ctx_t *ctx,
-                               const struct option *opt, int unset)
+ static enum parse_opt_result stdin_callback(
+       struct parse_opt_ctx_t *ctx, const struct option *opt,
+       const char *arg, int unset)
  {
        int *read_from_stdin = opt->value;
  
        BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
  
        if (ctx->argc != 1)
                return error("option '%s' must be the last argument", opt->long_name);
        return 0;
  }
  
- static int unresolve_callback(struct parse_opt_ctx_t *ctx,
-                               const struct option *opt, int unset)
+ static enum parse_opt_result unresolve_callback(
+       struct parse_opt_ctx_t *ctx, const struct option *opt,
+       const char *arg, int unset)
  {
        int *has_errors = opt->value;
        const char *prefix = startup_info->prefix;
  
        BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
  
        /* consume remaining arguments. */
        *has_errors = do_unresolve(ctx->argc, ctx->argv,
        return 0;
  }
  
- static int reupdate_callback(struct parse_opt_ctx_t *ctx,
-                               const struct option *opt, int unset)
+ static enum parse_opt_result reupdate_callback(
+       struct parse_opt_ctx_t *ctx, const struct option *opt,
+       const char *arg, int unset)
  {
        int *has_errors = opt->value;
        const char *prefix = startup_info->prefix;
  
        BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
  
        /* consume remaining arguments. */
        setup_work_tree();
@@@ -986,7 -995,8 +996,8 @@@ int cmd_update_index(int argc, const ch
                        N_("add the specified entry to the index"),
                        PARSE_OPT_NOARG | /* disallow --cacheinfo=<mode> form */
                        PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
-                       (parse_opt_cb *) cacheinfo_callback},
+                       NULL, 0,
+                       cacheinfo_callback},
                {OPTION_CALLBACK, 0, "chmod", &set_executable_bit, "(+|-)x",
                        N_("override the executable bit of the listed files"),
                        PARSE_OPT_NONEG,
                {OPTION_LOWLEVEL_CALLBACK, 0, "stdin", &read_from_stdin, NULL,
                        N_("read list of paths to be updated from standard input"),
                        PARSE_OPT_NONEG | PARSE_OPT_NOARG,
-                       (parse_opt_cb *) stdin_callback},
+                       NULL, 0, stdin_callback},
                {OPTION_LOWLEVEL_CALLBACK, 0, "index-info", &nul_term_line, NULL,
                        N_("add entries from standard input to the index"),
                        PARSE_OPT_NONEG | PARSE_OPT_NOARG,
-                       (parse_opt_cb *) stdin_cacheinfo_callback},
+                       NULL, 0, stdin_cacheinfo_callback},
                {OPTION_LOWLEVEL_CALLBACK, 0, "unresolve", &has_errors, NULL,
                        N_("repopulate stages #2 and #3 for the listed paths"),
                        PARSE_OPT_NONEG | PARSE_OPT_NOARG,
-                       (parse_opt_cb *) unresolve_callback},
+                       NULL, 0, unresolve_callback},
                {OPTION_LOWLEVEL_CALLBACK, 'g', "again", &has_errors, NULL,
                        N_("only update entries that differ from HEAD"),
                        PARSE_OPT_NONEG | PARSE_OPT_NOARG,
-                       (parse_opt_cb *) reupdate_callback},
+                       NULL, 0, reupdate_callback},
                OPT_BIT(0, "ignore-missing", &refresh_args.flags,
                        N_("ignore files missing from worktree"),
                        REFRESH_IGNORE_MISSING),
diff --combined diff.c
index 5306c48652db59e84c26383d68cf4a7d896647d4,4bc9df7362360651b5d871cc3cf8b653a42543d2..1a1f21578766e5fa6c455c94cce358ccad5de1d1
--- 1/diff.c
--- 2/diff.c
+++ b/diff.c
@@@ -23,6 -23,7 +23,7 @@@
  #include "argv-array.h"
  #include "graph.h"
  #include "packfile.h"
+ #include "parse-options.h"
  #include "help.h"
  
  #ifdef NO_FAST_WORKING_DIRECTORY
@@@ -304,9 -305,7 +305,9 @@@ static unsigned parse_color_moved_ws(co
                strbuf_addstr(&sb, i->string);
                strbuf_trim(&sb);
  
 -              if (!strcmp(sb.buf, "ignore-space-change"))
 +              if (!strcmp(sb.buf, "no"))
 +                      ret = 0;
 +              else if (!strcmp(sb.buf, "ignore-space-change"))
                        ret |= XDF_IGNORE_WHITESPACE_CHANGE;
                else if (!strcmp(sb.buf, "ignore-space-at-eol"))
                        ret |= XDF_IGNORE_WHITESPACE_AT_EOL;
  
        if ((ret & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) &&
            (ret & XDF_WHITESPACE_FLAGS)) {
 -              error(_("color-moved-ws: allow-indentation-change cannot be combined with other white space modes"));
 +              error(_("color-moved-ws: allow-indentation-change cannot be combined with other whitespace modes"));
                ret |= COLOR_MOVED_WS_ERROR;
        }
  
@@@ -495,7 -494,7 +496,7 @@@ static const char *external_diff(void
  
        if (done_preparing)
                return external_diff_cmd;
 -      external_diff_cmd = getenv("GIT_EXTERNAL_DIFF");
 +      external_diff_cmd = xstrdup_or_null(getenv("GIT_EXTERNAL_DIFF"));
        if (!external_diff_cmd)
                external_diff_cmd = external_diff_cmd_cfg;
        done_preparing = 1;
@@@ -756,8 -755,6 +757,8 @@@ struct emitted_diff_symbol 
        const char *line;
        int len;
        int flags;
 +      int indent_off;   /* Offset to first non-whitespace character */
 +      int indent_width; /* The visual width of the indentation */
        enum diff_symbol s;
  };
  #define EMITTED_DIFF_SYMBOL_INIT {NULL}
@@@ -788,85 -785,44 +789,85 @@@ struct moved_entry 
        struct moved_entry *next_line;
  };
  
 -/**
 - * The struct ws_delta holds white space differences between moved lines, i.e.
 - * between '+' and '-' lines that have been detected to be a move.
 - * The string contains the difference in leading white spaces, before the
 - * rest of the line is compared using the white space config for move
 - * coloring. The current_longer indicates if the first string in the
 - * comparision is longer than the second.
 - */
 -struct ws_delta {
 -      char *string;
 -      unsigned int current_longer : 1;
 -};
 -#define WS_DELTA_INIT { NULL, 0 }
 -
  struct moved_block {
        struct moved_entry *match;
 -      struct ws_delta wsd;
 +      int wsd; /* The whitespace delta of this block */
  };
  
  static void moved_block_clear(struct moved_block *b)
  {
 -      FREE_AND_NULL(b->wsd.string);
 -      b->match = NULL;
 +      memset(b, 0, sizeof(*b));
  }
  
 -static int compute_ws_delta(const struct emitted_diff_symbol *a,
 -                           const struct emitted_diff_symbol *b,
 -                           struct ws_delta *out)
 +#define INDENT_BLANKLINE INT_MIN
 +
 +static void fill_es_indent_data(struct emitted_diff_symbol *es)
  {
 -      const struct emitted_diff_symbol *longer =  a->len > b->len ? a : b;
 -      const struct emitted_diff_symbol *shorter = a->len > b->len ? b : a;
 -      int d = longer->len - shorter->len;
 +      unsigned int off = 0, i;
 +      int width = 0, tab_width = es->flags & WS_TAB_WIDTH_MASK;
 +      const char *s = es->line;
 +      const int len = es->len;
 +
 +      /* skip any \v \f \r at start of indentation */
 +      while (s[off] == '\f' || s[off] == '\v' ||
 +             (s[off] == '\r' && off < len - 1))
 +              off++;
 +
 +      /* calculate the visual width of indentation */
 +      while(1) {
 +              if (s[off] == ' ') {
 +                      width++;
 +                      off++;
 +              } else if (s[off] == '\t') {
 +                      width += tab_width - (width % tab_width);
 +                      while (s[++off] == '\t')
 +                              width += tab_width;
 +              } else {
 +                      break;
 +              }
 +      }
 +
 +      /* check if this line is blank */
 +      for (i = off; i < len; i++)
 +              if (!isspace(s[i]))
 +                  break;
 +
 +      if (i == len) {
 +              es->indent_width = INDENT_BLANKLINE;
 +              es->indent_off = len;
 +      } else {
 +              es->indent_off = off;
 +              es->indent_width = width;
 +      }
 +}
 +
 +static int compute_ws_delta(const struct emitted_diff_symbol *a,
 +                          const struct emitted_diff_symbol *b,
 +                          int *out)
 +{
 +      int a_len = a->len,
 +          b_len = b->len,
 +          a_off = a->indent_off,
 +          a_width = a->indent_width,
 +          b_off = b->indent_off,
 +          b_width = b->indent_width;
 +      int delta;
 +
 +      if (a_width == INDENT_BLANKLINE && b_width == INDENT_BLANKLINE) {
 +              *out = INDENT_BLANKLINE;
 +              return 1;
 +      }
  
 -      if (strncmp(longer->line + d, shorter->line, shorter->len))
 +      if (a->s == DIFF_SYMBOL_PLUS)
 +              delta = a_width - b_width;
 +      else
 +              delta = b_width - a_width;
 +
 +      if (a_len - a_off != b_len - b_off ||
 +          memcmp(a->line + a_off, b->line + b_off, a_len - a_off))
                return 0;
  
 -      out->string = xmemdupz(longer->line, d);
 -      out->current_longer = (a == longer);
 +      *out = delta;
  
        return 1;
  }
@@@ -878,53 -834,51 +879,53 @@@ static int cmp_in_block_with_wsd(const 
                                 int n)
  {
        struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n];
 -      int al = cur->es->len, cl = l->len;
 +      int al = cur->es->len, bl = match->es->len, cl = l->len;
        const char *a = cur->es->line,
                   *b = match->es->line,
                   *c = l->line;
 -
 -      int wslen;
 +      int a_off = cur->es->indent_off,
 +          a_width = cur->es->indent_width,
 +          c_off = l->indent_off,
 +          c_width = l->indent_width;
 +      int delta;
  
        /*
 -       * We need to check if 'cur' is equal to 'match'.
 -       * As those are from the same (+/-) side, we do not need to adjust for
 -       * indent changes. However these were found using fuzzy matching
 -       * so we do have to check if they are equal.
 +       * We need to check if 'cur' is equal to 'match'.  As those
 +       * are from the same (+/-) side, we do not need to adjust for
 +       * indent changes. However these were found using fuzzy
 +       * matching so we do have to check if they are equal. Here we
 +       * just check the lengths. We delay calling memcmp() to check
 +       * the contents until later as if the length comparison for a
 +       * and c fails we can avoid the call all together.
         */
 -      if (strcmp(a, b))
 +      if (al != bl)
                return 1;
  
 -      if (!pmb->wsd.string)
 -              /*
 -               * The white space delta is not active? This can happen
 -               * when we exit early in this function.
 -               */
 -              return 1;
 +      /* If 'l' and 'cur' are both blank then they match. */
 +      if (a_width == INDENT_BLANKLINE && c_width == INDENT_BLANKLINE)
 +              return 0;
  
        /*
 -       * The indent changes of the block are known and stored in
 -       * pmb->wsd; however we need to check if the indent changes of the
 -       * current line are still the same as before.
 -       *
 -       * To do so we need to compare 'l' to 'cur', adjusting the
 -       * one of them for the white spaces, depending which was longer.
 +       * The indent changes of the block are known and stored in pmb->wsd;
 +       * however we need to check if the indent changes of the current line
 +       * match those of the current block and that the text of 'l' and 'cur'
 +       * after the indentation match.
         */
 +      if (cur->es->s == DIFF_SYMBOL_PLUS)
 +              delta = a_width - c_width;
 +      else
 +              delta = c_width - a_width;
  
 -      wslen = strlen(pmb->wsd.string);
 -      if (pmb->wsd.current_longer) {
 -              c += wslen;
 -              cl -= wslen;
 -      } else {
 -              a += wslen;
 -              al -= wslen;
 -      }
 -
 -      if (al != cl || memcmp(a, c, al))
 -              return 1;
 +      /*
 +       * If the previous lines of this block were all blank then set its
 +       * whitespace delta.
 +       */
 +      if (pmb->wsd == INDENT_BLANKLINE)
 +              pmb->wsd = delta;
  
 -      return 0;
 +      return !(delta == pmb->wsd && al - a_off == cl - c_off &&
 +               !memcmp(a, b, al) && !
 +               memcmp(a + a_off, c + c_off, al - a_off));
  }
  
  static int moved_entry_cmp(const void *hashmap_cmp_fn_data,
@@@ -990,9 -944,6 +991,9 @@@ static void add_lines_to_move_detection
                        continue;
                }
  
 +              if (o->color_moved_ws_handling &
 +                  COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE)
 +                      fill_es_indent_data(&o->emitted_symbols->buf[n]);
                key = prepare_entry(o, n);
                if (prev_line && prev_line->es->s == o->emitted_symbols->buf[n].s)
                        prev_line->next_line = key;
@@@ -1071,7 -1022,8 +1072,7 @@@ static int shrink_potential_moved_block
  
                if (lp < pmb_nr && rp > -1 && lp < rp) {
                        pmb[lp] = pmb[rp];
 -                      pmb[rp].match = NULL;
 -                      pmb[rp].wsd.string = NULL;
 +                      memset(&pmb[rp], 0, sizeof(pmb[rp]));
                        rp--;
                        lp++;
                }
   * The last block consists of the (n - block_length)'th line up to but not
   * including the nth line.
   *
 + * Returns 0 if the last block is empty or is unset by this function, non zero
 + * otherwise.
 + *
   * NEEDSWORK: This uses the same heuristic as blame_entry_score() in blame.c.
   * Think of a way to unify them.
   */
 -static void adjust_last_block(struct diff_options *o, int n, int block_length)
 +static int adjust_last_block(struct diff_options *o, int n, int block_length)
  {
        int i, alnum_count = 0;
        if (o->color_moved == COLOR_MOVED_PLAIN)
 -              return;
 +              return block_length;
        for (i = 1; i < block_length + 1; i++) {
                const char *c = o->emitted_symbols->buf[n - i].line;
                for (; *c; c++) {
                                continue;
                        alnum_count++;
                        if (alnum_count >= COLOR_MOVED_MIN_ALNUM_COUNT)
 -                              return;
 +                              return 1;
                }
        }
        for (i = 1; i < block_length + 1; i++)
                o->emitted_symbols->buf[n - i].flags &= ~DIFF_SYMBOL_MOVED_LINE;
 +      return 0;
  }
  
  /* Find blocks of moved code, delegate actual coloring decision to helper */
@@@ -1124,7 -1072,7 +1125,7 @@@ static void mark_color_as_moved(struct 
  {
        struct moved_block *pmb = NULL; /* potentially moved blocks */
        int pmb_nr = 0, pmb_alloc = 0;
 -      int n, flipped_block = 1, block_length = 0;
 +      int n, flipped_block = 0, block_length = 0;
  
  
        for (n = 0; n < o->emitted_symbols->nr; n++) {
                struct moved_entry *key;
                struct moved_entry *match = NULL;
                struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n];
 +              enum diff_symbol last_symbol = 0;
  
                switch (l->s) {
                case DIFF_SYMBOL_PLUS:
                        free(key);
                        break;
                default:
 -                      flipped_block = 1;
 +                      flipped_block = 0;
                }
  
                if (!match) {
                                moved_block_clear(&pmb[i]);
                        pmb_nr = 0;
                        block_length = 0;
 +                      flipped_block = 0;
 +                      last_symbol = l->s;
                        continue;
                }
  
 -              l->flags |= DIFF_SYMBOL_MOVED_LINE;
 -
 -              if (o->color_moved == COLOR_MOVED_PLAIN)
 +              if (o->color_moved == COLOR_MOVED_PLAIN) {
 +                      last_symbol = l->s;
 +                      l->flags |= DIFF_SYMBOL_MOVED_LINE;
                        continue;
 +              }
  
                if (o->color_moved_ws_handling &
                    COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE)
                                                             &pmb[pmb_nr].wsd))
                                                pmb[pmb_nr++].match = match;
                                } else {
 -                                      pmb[pmb_nr].wsd.string = NULL;
 +                                      pmb[pmb_nr].wsd = 0;
                                        pmb[pmb_nr++].match = match;
                                }
                        }
  
 -                      flipped_block = (flipped_block + 1) % 2;
 +                      if (adjust_last_block(o, n, block_length) &&
 +                          pmb_nr && last_symbol != l->s)
 +                              flipped_block = (flipped_block + 1) % 2;
 +                      else
 +                              flipped_block = 0;
  
 -                      adjust_last_block(o, n, block_length);
                        block_length = 0;
                }
  
 -              block_length++;
 -
 -              if (flipped_block && o->color_moved != COLOR_MOVED_BLOCKS)
 -                      l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
 +              if (pmb_nr) {
 +                      block_length++;
 +                      l->flags |= DIFF_SYMBOL_MOVED_LINE;
 +                      if (flipped_block && o->color_moved != COLOR_MOVED_BLOCKS)
 +                              l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
 +              }
 +              last_symbol = l->s;
        }
        adjust_last_block(o, n, block_length);
  
@@@ -1555,7 -1493,7 +1556,7 @@@ static void emit_diff_symbol_from_struc
  static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,
                             const char *line, int len, unsigned flags)
  {
 -      struct emitted_diff_symbol e = {line, len, flags, s};
 +      struct emitted_diff_symbol e = {line, len, flags, 0, 0, s};
  
        if (o->emitted_symbols)
                append_emitted_diff_symbol(o, &e);
@@@ -3544,7 -3482,7 +3545,7 @@@ static void builtin_diff(const char *na
                o->found_changes = 1;
        } else {
                /* Crazy xdl interfaces.. */
 -              const char *diffopts = getenv("GIT_DIFF_OPTS");
 +              const char *diffopts;
                const char *v;
                xpparam_t xpp;
                xdemitconf_t xecfg;
                        xecfg.flags |= XDL_EMIT_FUNCCONTEXT;
                if (pe)
                        xdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);
 +
 +              diffopts = getenv("GIT_DIFF_OPTS");
                if (!diffopts)
                        ;
                else if (skip_prefix(diffopts, "--unified=", &v))
                        xecfg.ctxlen = strtoul(v, NULL, 10);
                else if (skip_prefix(diffopts, "-u", &v))
                        xecfg.ctxlen = strtoul(v, NULL, 10);
 +
                if (o->word_diff)
                        init_diff_words_data(&ecbdata, o, one, two);
                if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume,
@@@ -4491,6 -4426,8 +4492,8 @@@ static void run_checkdiff(struct diff_f
        builtin_checkdiff(name, other, attr_path, p->one, p->two, o);
  }
  
+ static void prep_parse_options(struct diff_options *options);
  void repo_diff_setup(struct repository *r, struct diff_options *options)
  {
        memcpy(options, &default_diff_options, sizeof(*options));
  
        options->color_moved = diff_color_moved_default;
        options->color_moved_ws_handling = diff_color_moved_ws_default;
+       prep_parse_options(options);
  }
  
  void diff_setup_done(struct diff_options *options)
  
        if (!options->use_color || external_diff())
                options->color_moved = 0;
+       FREE_AND_NULL(options->parseopts);
  }
  
  static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
@@@ -4926,6 -4867,47 +4933,47 @@@ static int parse_objfind_opt(struct dif
        return 1;
  }
  
+ static int diff_opt_unified(const struct option *opt,
+                           const char *arg, int unset)
+ {
+       struct diff_options *options = opt->value;
+       char *s;
+       BUG_ON_OPT_NEG(unset);
+       options->context = strtol(arg, &s, 10);
+       if (*s)
+               return error(_("%s expects a numerical value"), "--unified");
+       enable_patch_output(&options->output_format);
+       return 0;
+ }
+ static void prep_parse_options(struct diff_options *options)
+ {
+       struct option parseopts[] = {
+               OPT_GROUP(N_("Diff output format options")),
+               OPT_BITOP('p', "patch", &options->output_format,
+                         N_("generate patch"),
+                         DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),
+               OPT_BITOP('u', NULL, &options->output_format,
+                         N_("generate patch"),
+                         DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),
+               OPT_CALLBACK_F('U', "unified", options, N_("<n>"),
+                              N_("generate diffs with <n> lines context"),
+                              PARSE_OPT_NONEG, diff_opt_unified),
+               OPT_BOOL('W', "function-context", &options->flags.funccontext,
+                        N_("generate diffs with <n> lines context")),
+               OPT_BIT_F(0, "raw", &options->output_format,
+                         N_("generate the diff in raw format"),
+                         DIFF_FORMAT_RAW, PARSE_OPT_NONEG),
+               OPT_END()
+       };
+       ALLOC_ARRAY(options->parseopts, ARRAY_SIZE(parseopts));
+       memcpy(options->parseopts, parseopts, sizeof(parseopts));
+ }
  int diff_opt_parse(struct diff_options *options,
                   const char **av, int ac, const char *prefix)
  {
        if (!prefix)
                prefix = "";
  
+       ac = parse_options(ac, av, prefix, options->parseopts, NULL,
+                          PARSE_OPT_KEEP_DASHDASH |
+                          PARSE_OPT_KEEP_UNKNOWN |
+                          PARSE_OPT_NO_INTERNAL_HELP |
+                          PARSE_OPT_ONE_SHOT |
+                          PARSE_OPT_STOP_AT_NON_OPTION);
+       if (ac)
+               return ac;
        /* Output format options */
-       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")) {
+       if (!strcmp(arg, "--patch-with-raw")) {
                enable_patch_output(&options->output_format);
                options->output_format |= DIFF_FORMAT_RAW;
        } else if (!strcmp(arg, "--numstat"))
                if (cm < 0)
                        return error("bad --color-moved argument: %s", arg);
                options->color_moved = cm;
 +      } else if (!strcmp(arg, "--no-color-moved-ws")) {
 +              options->color_moved_ws_handling = 0;
        } else if (skip_prefix(arg, "--color-moved-ws=", &arg)) {
                unsigned cm = parse_color_moved_ws(arg);
                if (cm & COLOR_MOVED_WS_ERROR)
        else if (opt_arg(arg, '\0', "inter-hunk-context",
                         &options->interhunkcontext))
                ;
-       else if (!strcmp(arg, "-W"))
-               options->flags.funccontext = 1;
-       else if (!strcmp(arg, "--function-context"))
-               options->flags.funccontext = 1;
-       else if (!strcmp(arg, "--no-function-context"))
-               options->flags.funccontext = 0;
        else if ((argcount = parse_long_opt("output", av, &optarg))) {
                char *path = prefix_filename(prefix, optarg);
                options->file = xfopen(path, "w");
@@@ -5962,10 -5941,8 +6009,10 @@@ static void diff_flush_patch_all_file_p
  
                for (i = 0; i < esm.nr; i++)
                        free((void *)esm.buf[i].line);
 +              esm.nr = 0;
 +
 +              o->emitted_symbols = NULL;
        }
 -      esm.nr = 0;
  }
  
  void diff_flush(struct diff_options *options)