From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 15 Sep 2009 10:28:08 +0000 (-0700)
Subject: Merge branch 'jc/maint-1.6.0-blank-at-eof' (early part) into jc/maint-blank-at-eof
X-Git-Tag: v1.6.5.3~11^2~3
X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=afd9db4173b99fb2fb103ec604498896a6bedaa7;p=git.git

Merge branch 'jc/maint-1.6.0-blank-at-eof' (early part) into jc/maint-blank-at-eof

* '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
---

afd9db4173b99fb2fb103ec604498896a6bedaa7
diff --cc diff.c
index cd35e0c2d,63a3bfc33..87a3e8956
--- a/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;
  	}
  
 +	if (diff_suppress_blank_empty
 +	    && len == 2 && line[0] == ' ' && line[1] == '\n') {
 +		line[0] = '\n';
 +		len = 1;
 +	}
 +
- 	/* 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 (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:
diff --cc t/t4124-apply-ws-rule.sh
index fac2093d7,778d45bc0..ca2639759
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@@ -148,26 -148,96 +148,117 @@@ d
  	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
index 59d0883c1,cd03bc032..760b5743f
--- a/ws.c
+++ b/ws.c
@@@ -10,12 -10,13 +10,14 @@@
  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--;
  			}