public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Thell Fowler <git@tbfowler•name>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 0/9] War on blank-at-eof
Date: Sat, 05 Sep 2009 23:13:51 -0700	[thread overview]
Message-ID: <7v63bwob1c.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: alpine.WNT.2.00.0909051534380.7040@GWNotebook

Thell Fowler <git@tbfowler•name> writes:

> While thinking about what appeared in:
>
> http://article.gmane.org/gmane.comp.version-control.git/124138

Oh, I forgot all about that one.  The suggestion does include two very
good points, one being "git apply" which I did, and the other being what I
completely forgot.  Introduction of blank-at-eol and blank-at-eof, and
make trailing-space a convenience synonym that triggers both.

Thanks for a reminder.  The following patch can come on top of the
series.

-- >8 --
Subject: core.whitespace: split trailing-space into blank-at-{eol,eof}

People who configured trailing-space depended on it to catch both extra
white space at the end of line, and extra blank lines at the end of file.
Earlier attempt to introduce only blank-at-eof gave them an escape hatch
to keep the old behaviour, but it is a regression until they explicitly
specify the new error class.

This introduces a blank-at-eol that only catches extra white space at the
end of line, and makes the traditional trailing-space a convenient synonym
to catch both blank-at-eol and blank-at-eof.  This way, people who used
trailing-space continue to catch both classes of errors.

Signed-off-by: Junio C Hamano <gitster@pobox•com>
---
 Documentation/config.txt |    5 ++++-
 cache.h                  |    5 +++--
 ws.c                     |   24 +++++++++++++++---------
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 871384e..0e245a7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -382,7 +382,7 @@ core.whitespace::
 	consider them as errors.  You can prefix `-` to disable
 	any of them (e.g. `-trailing-space`):
 +
-* `trailing-space` treats trailing whitespaces at the end of the line
+* `blank-at-eol` treats trailing whitespaces at the end of the line
   as an error (enabled by default).
 * `space-before-tab` treats a space character that appears immediately
   before a tab character in the initial indent part of the line as an
@@ -391,11 +391,14 @@ core.whitespace::
   space characters as an error (not enabled by default).
 * `blank-at-eof` treats blank lines added at the end of file as an error
   (enabled by default).
+* `trailing-space` is a short-hand to cover both `blank-at-eol` and
+  `blank-at-eof`.
 * `cr-at-eol` treats a carriage-return at the end of line as
   part of the line terminator, i.e. with it, `trailing-space`
   does not trigger if the character before such a carriage-return
   is not a whitespace (not enabled by default).
 
+
 core.fsyncobjectfiles::
 	This boolean will enable 'fsync()' when writing object files.
 +
diff --git a/cache.h b/cache.h
index 7152fea..ee12e74 100644
--- a/cache.h
+++ b/cache.h
@@ -841,12 +841,13 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
  * whitespace rules.
  * used by both diff and apply
  */
-#define WS_TRAILING_SPACE	01
+#define WS_BLANK_AT_EOL         01
 #define WS_SPACE_BEFORE_TAB	02
 #define WS_INDENT_WITH_NON_TAB	04
 #define WS_CR_AT_EOL           010
 #define WS_BLANK_AT_EOF        020
-#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|WS_BLANK_AT_EOF)
+#define WS_TRAILING_SPACE      (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)
+#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)
 extern unsigned whitespace_rule_cfg;
 extern unsigned whitespace_rule(const char *);
 extern unsigned parse_whitespace_rule(const char *);
diff --git a/ws.c b/ws.c
index d56636b..cd03bc0 100644
--- a/ws.c
+++ b/ws.c
@@ -15,6 +15,7 @@ static struct whitespace_rule {
 	{ "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 },
 };
 
@@ -101,9 +102,19 @@ unsigned whitespace_rule(const char *pathname)
 char *whitespace_error_string(unsigned ws)
 {
 	struct strbuf err;
+
 	strbuf_init(&err, 0);
-	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, ", ");
@@ -114,11 +125,6 @@ char *whitespace_error_string(unsigned ws)
 			strbuf_addstr(&err, ", ");
 		strbuf_addstr(&err, "indent with spaces");
 	}
-	if (ws & WS_BLANK_AT_EOF) {
-		if (err.len)
-			strbuf_addstr(&err, ", ");
-		strbuf_addstr(&err, "new blank line at EOF");
-	}
 	return strbuf_detach(&err, NULL);
 }
 
@@ -146,11 +152,11 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule,
 	}
 
 	/* Check for trailing whitespace. */
-	if (ws_rule & WS_TRAILING_SPACE) {
+	if (ws_rule & WS_BLANK_AT_EOL) {
 		for (i = len - 1; i >= 0; i--) {
 			if (isspace(line[i])) {
 				trailing_whitespace = i;
-				result |= WS_TRAILING_SPACE;
+				result |= WS_BLANK_AT_EOL;
 			}
 			else
 				break;
@@ -266,7 +272,7 @@ int ws_fix_copy(char *dst, const char *src, int len, unsigned ws_rule, int *erro
 	/*
 	 * 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') {
 			add_nl_to_tail = 1;
-- 
1.6.4.2.313.g0425f

      reply	other threads:[~2009-09-06  6:14 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-04 10:55 [PATCH 0/9] War on blank-at-eof Junio C Hamano
2009-09-04 10:55 ` [PATCH 1/9] apply --whitespace=fix: fix handling of blank lines at the eof Junio C Hamano
2009-09-04 10:55 ` [PATCH 2/9] apply --whitespace=fix: detect new blank lines at eof correctly Junio C Hamano
2009-09-04 12:02   ` Johannes Sixt
2009-09-04 16:26     ` Junio C Hamano
2009-09-04 10:55 ` [PATCH 3/9] apply.c: split check_whitespace() into two Junio C Hamano
2009-09-04 10:55 ` [PATCH 4/9] apply --whitespace=warn/error: diagnose blank at EOF Junio C Hamano
2009-09-04 10:55 ` [PATCH 5/9] apply --whitespace: warn blank but not necessarily empty lines " Junio C Hamano
2009-09-04 10:55 ` [PATCH 6/9] diff.c: the builtin_diff() deals with only two-file comparison Junio C Hamano
2009-09-04 10:55 ` [PATCH 7/9] diff --whitespace=warn/error: obey blank-at-eof Junio C Hamano
2009-09-04 10:55 ` [PATCH 8/9] diff --whitespace=warn/error: fix blank-at-eof check Junio C Hamano
2009-09-04 10:55 ` [PATCH 9/9] diff --color: color blank-at-eof Junio C Hamano
2009-09-05 21:28 ` [PATCH 0/9] War on blank-at-eof Thell Fowler
2009-09-06  6:13   ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7v63bwob1c.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox$(echo .)com \
    --cc=git@tbfowler$(echo .)name \
    --cc=git@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox