From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Tue, 06 Mar 2007 22:19:13 GMT
Subject: Re: [PATCH 3/5] restrict the patch filtering
Message-ID: <Pine.LNX.4.64.0703061406580.5963@woody.linux-foundation.org>
In-Reply-To: <117321826466-git-send-email-dzickus@redhat.com>



On Tue, 6 Mar 2007, Don Zickus wrote:
>
> I have come across many emails that use long strings of '-'s as separators
> for ideas.  This patch below limits the separator to only 3 '-', with the
> intent that long string of '-'s will stay in the commit msg and not in the
> patch file.

Ack. I think this is better than what we have now. My only question is 
whether we should allow arbitrary whitespace after the "---", ie all of 
space/tab/newline. Your patch just does space/newline (and only _one_ 
space).

I think the space-only is probably fine - we don't allow tabs there in 
diffs, and the only reason I mention whitespace is that in an editor it's 
actually really hard to see spaces at the ends of lines, so I think it's 
bad form to behave differently for

	"---\n"

than for

	"--- \t \n"

when they both *look* the same in an editor.

> Signed-off-by: Don Zickus <dzickus@redhat.com>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>

Although I'd be even happier with something like

	static inline int patchbreak(const char *line)
	{
		/* Beginning of a "diff -" header? */
		if (!memcmp("diff -", line, 6))
			return 1;

		/* CVS "Index: " line? */
		if (!memcmp("Index: ", line, 7))
			return 1;

		/*
		 * "--- <filename>" starts patches without headers
		 * "---<sp>*" is a manual separator
		 */
		if (!memcmp("---", line, 3)) {
			line += 3;
			/* space followed by a filename? */
			if (line[0] == ' ' && !isspace(line[1]))
				return 1;
			/* Just whitespace? */
			for (;;) {
				unsigned char c = *line++;
				if (c == '\n')
					return 1;
				if (!isspace(c))
					break;
			}
			return 0;
		}
		return 0;
	}

which is totally untested, of course.

		Linus