Merge branch 'jc/maint-1.6.0-blank-at-eof' (early part) into jc/maint-blank-at-eof
authorJunio C Hamano <gitster@pobox.com>
Tue, 15 Sep 2009 10:28:08 +0000 (03:28 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 15 Sep 2009 10:28:08 +0000 (03:28 -0700)
* 'jc/maint-1.6.0-blank-at-eof' (early part):
  diff --whitespace: fix blank lines at end
  core.whitespace: split trailing-space into blank-at-{eol,eof}
  diff --color: color blank-at-eof
  diff --whitespace=warn/error: fix blank-at-eof check
  diff --whitespace=warn/error: obey blank-at-eof
  diff.c: the builtin_diff() deals with only two-file comparison
  apply --whitespace: warn blank but not necessarily empty lines at EOF
  apply --whitespace=warn/error: diagnose blank at EOF
  apply.c: split check_whitespace() into two
  apply --whitespace=fix: detect new blank lines at eof correctly
  apply --whitespace=fix: fix handling of blank lines at the eof

1  2 
Documentation/config.txt
builtin-apply.c
cache.h
diff.c
t/t4015-diff-whitespace.sh
t/t4124-apply-ws-rule.sh
ws.c

Simple merge
diff --cc builtin-apply.c
Simple merge
diff --cc cache.h
Simple merge
diff --cc diff.c
--- 1/diff.c
--- 2/diff.c
+++ b/diff.c
@@@ -532,8 -488,13 +532,12 @@@ static void diff_words_show(struct diff
  typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
  
  struct emit_callback {
-       int nparents, color_diff;
 -      struct xdiff_emit_state xm;
+       int color_diff;
        unsigned ws_rule;
+       int blank_at_eof_in_preimage;
+       int blank_at_eof_in_postimage;
+       int lno_in_preimage;
+       int lno_in_postimage;
        sane_truncate_fn truncate;
        const char **label_path;
        struct diff_words_data *diff_words;
@@@ -644,20 -628,9 +674,15 @@@ static void fn_out_consume(void *priv, 
                ecbdata->label_path[0] = ecbdata->label_path[1] = NULL;
        }
  
-       /* This is not really necessary for now because
-        * this codepath only deals with two-way diffs.
-        */
-       for (i = 0; i < len && line[i] == '@'; i++)
-               ;
-       if (2 <= i && i < len && line[i] == ' ') {
-               ecbdata->nparents = i - 1;
 +      if (diff_suppress_blank_empty
 +          && len == 2 && line[0] == ' ' && line[1] == '\n') {
 +              line[0] = '\n';
 +              len = 1;
 +      }
 +
+       if (line[0] == '@') {
                len = sane_truncate_line(ecbdata, line, len);
+               find_lno(line, ecbdata);
                emit_line(ecbdata->file,
                          diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),
                          reset, line, len);
@@@ -1380,63 -1345,170 +1393,108 @@@ static void emit_binary_diff(FILE *file
        emit_binary_diff_body(file, two, one);
  }
  
 -static void setup_diff_attr_check(struct git_attr_check *check)
 +static void diff_filespec_load_driver(struct diff_filespec *one)
  {
 -      static struct git_attr *attr_diff;
 -
 -      if (!attr_diff) {
 -              attr_diff = git_attr("diff", 4);
 -      }
 -      check[0].attr = attr_diff;
 +      if (!one->driver)
 +              one->driver = userdiff_find_by_path(one->path);
 +      if (!one->driver)
 +              one->driver = userdiff_find_by_name("default");
  }
  
 -static void diff_filespec_check_attr(struct diff_filespec *one)
 +int diff_filespec_is_binary(struct diff_filespec *one)
  {
 -      struct git_attr_check attr_diff_check;
 -      int check_from_data = 0;
 -
 -      if (one->checked_attr)
 -              return;
 -
 -      setup_diff_attr_check(&attr_diff_check);
 -      one->is_binary = 0;
 -      one->funcname_pattern_ident = NULL;
 -
 -      if (!git_checkattr(one->path, 1, &attr_diff_check)) {
 -              const char *value;
 -
 -              /* binaryness */
 -              value = attr_diff_check.value;
 -              if (ATTR_TRUE(value))
 -                      ;
 -              else if (ATTR_FALSE(value))
 -                      one->is_binary = 1;
 -              else
 -                      check_from_data = 1;
 -
 -              /* funcname pattern ident */
 -              if (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value))
 -                      ;
 -              else
 -                      one->funcname_pattern_ident = value;
 -      }
 -
 -      if (check_from_data) {
 -              if (!one->data && DIFF_FILE_VALID(one))
 -                      diff_populate_filespec(one, 0);
 -
 -              if (one->data)
 -                      one->is_binary = buffer_is_binary(one->data, one->size);
 +      if (one->is_binary == -1) {
 +              diff_filespec_load_driver(one);
 +              if (one->driver->binary != -1)
 +                      one->is_binary = one->driver->binary;
 +              else {
 +                      if (!one->data && DIFF_FILE_VALID(one))
 +                              diff_populate_filespec(one, 0);
 +                      if (one->data)
 +                              one->is_binary = buffer_is_binary(one->data,
 +                                              one->size);
 +                      if (one->is_binary == -1)
 +                              one->is_binary = 0;
 +              }
        }
 +      return one->is_binary;
  }
  
 -int diff_filespec_is_binary(struct diff_filespec *one)
 +static const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespec *one)
  {
 -      diff_filespec_check_attr(one);
 -      return one->is_binary;
 +      diff_filespec_load_driver(one);
 +      return one->driver->funcname.pattern ? &one->driver->funcname : NULL;
  }
  
 -static const struct funcname_pattern_entry *funcname_pattern(const char *ident)
 -{
 -      struct funcname_pattern_list *pp;
 -
 -      for (pp = funcname_pattern_list; pp; pp = pp->next)
 -              if (!strcmp(ident, pp->e.name))
 -                      return &pp->e;
 -      return NULL;
 -}
 -
 -static const struct funcname_pattern_entry builtin_funcname_pattern[] = {
 -      { "java",
 -        "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
 -        "^[ \t]*(([ \t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \t]*\\([^;]*)$",
 -        REG_EXTENDED },
 -      { "pascal",
 -        "^((procedure|function|constructor|destructor|interface|"
 -              "implementation|initialization|finalization)[ \t]*.*)$"
 -        "|"
 -        "^(.*=[ \t]*(class|record).*)$",
 -        REG_EXTENDED },
 -      { "bibtex", "(@[a-zA-Z]{1,}[ \t]*\\{{0,1}[ \t]*[^ \t\"@',\\#}{~%]*).*$",
 -        REG_EXTENDED },
 -      { "tex",
 -        "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$",
 -        REG_EXTENDED },
 -      { "ruby", "^[ \t]*((class|module|def)[ \t].*)$",
 -        REG_EXTENDED },
 -};
 -
 -static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_filespec *one)
 +static const char *userdiff_word_regex(struct diff_filespec *one)
  {
 -      const char *ident;
 -      const struct funcname_pattern_entry *pe;
 -      int i;
 -
 -      diff_filespec_check_attr(one);
 -      ident = one->funcname_pattern_ident;
 -
 -      if (!ident)
 -              /*
 -               * If the config file has "funcname.default" defined, that
 -               * regexp is used; otherwise NULL is returned and xemit uses
 -               * the built-in default.
 -               */
 -              return funcname_pattern("default");
 -
 -      /* Look up custom "funcname.$ident" regexp from config. */
 -      pe = funcname_pattern(ident);
 -      if (pe)
 -              return pe;
 +      diff_filespec_load_driver(one);
 +      return one->driver->word_regex;
 +}
  
 -      /*
 -       * And define built-in fallback patterns here.  Note that
 -       * these can be overridden by the user's config settings.
 -       */
 -      for (i = 0; i < ARRAY_SIZE(builtin_funcname_pattern); i++)
 -              if (!strcmp(ident, builtin_funcname_pattern[i].name))
 -                      return &builtin_funcname_pattern[i];
 +void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b)
 +{
 +      if (!options->a_prefix)
 +              options->a_prefix = a;
 +      if (!options->b_prefix)
 +              options->b_prefix = b;
 +}
  
 -      return NULL;
 +static const char *get_textconv(struct diff_filespec *one)
 +{
 +      if (!DIFF_FILE_VALID(one))
 +              return NULL;
 +      if (!S_ISREG(one->mode))
 +              return NULL;
 +      diff_filespec_load_driver(one);
 +      return one->driver->textconv;
  }
  
+ static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)
+ {
+       char *ptr = mf->ptr;
+       long size = mf->size;
+       int cnt = 0;
+       if (!size)
+               return cnt;
+       ptr += size - 1; /* pointing at the very end */
+       if (*ptr != '\n')
+               ; /* incomplete line */
+       else
+               ptr--; /* skip the last LF */
+       while (mf->ptr < ptr) {
+               char *prev_eol;
+               for (prev_eol = ptr; mf->ptr <= prev_eol; prev_eol--)
+                       if (*prev_eol == '\n')
+                               break;
+               if (!ws_blank_line(prev_eol + 1, ptr - prev_eol, ws_rule))
+                       break;
+               cnt++;
+               ptr = prev_eol - 1;
+       }
+       return cnt;
+ }
+ static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
+                              struct emit_callback *ecbdata)
+ {
+       int l1, l2, at;
+       unsigned ws_rule = ecbdata->ws_rule;
+       l1 = count_trailing_blank(mf1, ws_rule);
+       l2 = count_trailing_blank(mf2, ws_rule);
+       if (l2 <= l1) {
+               ecbdata->blank_at_eof_in_preimage = 0;
+               ecbdata->blank_at_eof_in_postimage = 0;
+               return;
+       }
+       at = count_lines(mf1->ptr, mf1->size);
+       ecbdata->blank_at_eof_in_preimage = (at - l1) + 1;
+       at = count_lines(mf2->ptr, mf2->size);
+       ecbdata->blank_at_eof_in_postimage = (at - l2) + 1;
+ }
  static void builtin_diff(const char *name_a,
                         const char *name_b,
                         struct diff_filespec *one,
@@@ -1701,14 -1720,26 +1761,25 @@@ static void builtin_checkdiff(const cha
                memset(&xecfg, 0, sizeof(xecfg));
                xecfg.ctxlen = 1; /* at least one context line */
                xpp.flags = XDF_NEED_MINIMAL;
 -              ecb.outf = xdiff_outf;
 -              ecb.priv = &data;
 -              xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
 +              xdi_diff_outf(&mf1, &mf2, checkdiff_consume, &data,
 +                            &xpp, &xecfg, &ecb);
  
-               if ((data.ws_rule & WS_TRAILING_SPACE) &&
-                   data.trailing_blanks_start) {
-                       fprintf(o->file, "%s:%d: ends with blank lines.\n",
-                               data.filename, data.trailing_blanks_start);
-                       data.status = 1; /* report errors */
+               if (data.ws_rule & WS_BLANK_AT_EOF) {
+                       struct emit_callback ecbdata;
+                       int blank_at_eof;
+                       ecbdata.ws_rule = data.ws_rule;
+                       check_blank_at_eof(&mf1, &mf2, &ecbdata);
+                       blank_at_eof = ecbdata.blank_at_eof_in_preimage;
+                       if (blank_at_eof) {
+                               static char *err;
+                               if (!err)
+                                       err = whitespace_error_string(WS_BLANK_AT_EOF);
+                               fprintf(o->file, "%s:%d: %s.\n",
+                                       data.filename, blank_at_eof, err);
+                               data.status = 1; /* report errors */
+                       }
                }
        }
   free_and_return:
Simple merge
        done
  done
  
 +create_patch () {
 +      sed -e "s/_/ /" <<-\EOF
 +              diff --git a/target b/target
 +              index e69de29..8bd6648 100644
 +              --- a/target
 +              +++ b/target
 +              @@ -0,0 +1,3 @@
 +              +An empty line follows
 +              +
 +              +A line with trailing whitespace and no newline_
 +              \ No newline at end of file
 +      EOF
 +}
 +
 +test_expect_success 'trailing whitespace & no newline at the end of file' '
 +      >target &&
 +      create_patch >patch-file &&
 +      git apply --whitespace=fix patch-file &&
 +      grep "newline$" target &&
 +      grep "^$" target
 +'
  
+ test_expect_success 'blank at EOF with --whitespace=fix (1)' '
+       : these can fail depending on what we did before
+       git config --unset core.whitespace
+       rm -f .gitattributes
+       { echo a; echo b; echo c; } >one &&
+       git add one &&
+       { echo a; echo b; echo c; } >expect &&
+       { cat expect; echo; } >one &&
+       git diff -- one >patch &&
+       git checkout one &&
+       git apply --whitespace=fix patch &&
+       test_cmp expect one
+ '
+ test_expect_success 'blank at EOF with --whitespace=fix (2)' '
+       { echo a; echo b; echo c; } >one &&
+       git add one &&
+       { echo a; echo c; } >expect &&
+       { cat expect; echo; echo; } >one &&
+       git diff -- one >patch &&
+       git checkout one &&
+       git apply --whitespace=fix patch &&
+       test_cmp expect one
+ '
+ test_expect_success 'blank at EOF with --whitespace=fix (3)' '
+       { echo a; echo b; echo; } >one &&
+       git add one &&
+       { echo a; echo c; echo; } >expect &&
+       { cat expect; echo; echo; } >one &&
+       git diff -- one >patch &&
+       git checkout one &&
+       git apply --whitespace=fix patch &&
+       test_cmp expect one
+ '
+ test_expect_success 'blank at end of hunk, not at EOF with --whitespace=fix' '
+       { echo a; echo b; echo; echo; echo; echo; echo; echo d; } >one &&
+       git add one &&
+       { echo a; echo c; echo; echo; echo; echo; echo; echo; echo d; } >expect &&
+       cp expect one &&
+       git diff -- one >patch &&
+       git checkout one &&
+       git apply --whitespace=fix patch &&
+       test_cmp expect one
+ '
+ test_expect_success 'blank at EOF with --whitespace=warn' '
+       { echo a; echo b; echo c; } >one &&
+       git add one &&
+       echo >>one &&
+       cat one >expect &&
+       git diff -- one >patch &&
+       git checkout one &&
+       git apply --whitespace=warn patch 2>error &&
+       test_cmp expect one &&
+       grep "new blank line at EOF" error
+ '
+ test_expect_success 'blank at EOF with --whitespace=error' '
+       { echo a; echo b; echo c; } >one &&
+       git add one &&
+       cat one >expect &&
+       echo >>one &&
+       git diff -- one >patch &&
+       git checkout one &&
+       test_must_fail git apply --whitespace=error patch 2>error &&
+       test_cmp expect one &&
+       grep "new blank line at EOF" error
+ '
+ test_expect_success 'blank but not empty at EOF' '
+       { echo a; echo b; echo c; } >one &&
+       git add one &&
+       echo "   " >>one &&
+       cat one >expect &&
+       git diff -- one >patch &&
+       git checkout one &&
+       git apply --whitespace=warn patch 2>error &&
+       test_cmp expect one &&
+       grep "new blank line at EOF" error
+ '
  test_done
diff --cc ws.c
--- 1/ws.c
--- 2/ws.c
+++ b/ws.c
  static struct whitespace_rule {
        const char *rule_name;
        unsigned rule_bits;
 +      unsigned loosens_error;
  } whitespace_rule_names[] = {
 -      { "trailing-space", WS_TRAILING_SPACE },
 -      { "space-before-tab", WS_SPACE_BEFORE_TAB },
 -      { "indent-with-non-tab", WS_INDENT_WITH_NON_TAB },
 -      { "cr-at-eol", WS_CR_AT_EOL },
 -      { "blank-at-eol", WS_BLANK_AT_EOL },
 -      { "blank-at-eof", WS_BLANK_AT_EOF },
 +      { "trailing-space", WS_TRAILING_SPACE, 0 },
 +      { "space-before-tab", WS_SPACE_BEFORE_TAB, 0 },
 +      { "indent-with-non-tab", WS_INDENT_WITH_NON_TAB, 0 },
 +      { "cr-at-eol", WS_CR_AT_EOL, 1 },
++      { "blank-at-eol", WS_BLANK_AT_EOL, 0 },
++      { "blank-at-eof", WS_BLANK_AT_EOF, 0 },
  };
  
  unsigned parse_whitespace_rule(const char *string)
@@@ -101,9 -101,20 +103,18 @@@ unsigned whitespace_rule(const char *pa
  /* The returned string should be freed by the caller. */
  char *whitespace_error_string(unsigned ws)
  {
 -      struct strbuf err;
 -
 -      strbuf_init(&err, 0);
 +      struct strbuf err = STRBUF_INIT;
-       if (ws & WS_TRAILING_SPACE)
+       if ((ws & WS_TRAILING_SPACE) == WS_TRAILING_SPACE)
                strbuf_addstr(&err, "trailing whitespace");
+       else {
+               if (ws & WS_BLANK_AT_EOL)
+                       strbuf_addstr(&err, "trailing whitespace");
+               if (ws & WS_BLANK_AT_EOF) {
+                       if (err.len)
+                               strbuf_addstr(&err, ", ");
+                       strbuf_addstr(&err, "new blank line at EOF");
+               }
+       }
        if (ws & WS_SPACE_BEFORE_TAB) {
                if (err.len)
                        strbuf_addstr(&err, ", ");
@@@ -261,11 -272,12 +272,11 @@@ int ws_fix_copy(char *dst, const char *
        /*
         * Strip trailing whitespace
         */
-       if (ws_rule & WS_TRAILING_SPACE) {
 -      if ((ws_rule & WS_BLANK_AT_EOL) &&
 -          (2 <= len && isspace(src[len-2]))) {
 -              if (src[len - 1] == '\n') {
++      if (ws_rule & WS_BLANK_AT_EOL) {
 +              if (0 < len && src[len - 1] == '\n') {
                        add_nl_to_tail = 1;
                        len--;
 -                      if (1 < len && src[len - 1] == '\r') {
 +                      if (0 < len && src[len - 1] == '\r') {
                                add_cr_to_tail = !!(ws_rule & WS_CR_AT_EOL);
                                len--;
                        }