threads / patch / 6506

patchdiff --check: use colour

Subject: [PATCH] diff --check: use colour

## tl;dr

26 messages between Jan 24, 2007 and Feb 18, 2007. Diffs are folded; open one to read it.

replies: 25people: 8as markdown or json

Johannes Schindelin· Jan 24, 2007, 14:05 UTC · lore
Reuse the colour handling of the regular diff.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
	Is it possible that diff_get_color() _never_ returns NULL?
	Then diff.c can be further cleaned up...
 diff.c |   58 ++++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 40 insertions(+), 18 deletions(-)
Show changes to diff.c +40 −18
diff --git a/diff.c b/diff.c
index f11a633..1b31c4f 100644
--- a/diff.c
+++ b/diff.c
@@ -405,22 +405,16 @@ static void emit_line(const char *set, const char *reset, const char *line, int
 	puts(reset);
 }
 
-static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
+static void emit_line_with_ws(int nparents,
+		const char *set, const char *reset, const char *ws,
+		const char *line, int len)
 {
-	int col0 = ecbdata->nparents;
+	int col0 = nparents;
 	int last_tab_in_indent = -1;
 	int last_space_in_indent = -1;
 	int i;
 	int tail = len;
 	int need_highlight_leading_space = 0;
-	const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
-	const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW);
-
-	if (!*ws) {
-		emit_line(set, reset, line, len);
-		return;
-	}
-
 	/* The line is a newly added line.  Does it have funny leading
 	 * whitespaces?  In indent, SP should never precede a TAB.
 	 */
@@ -475,6 +469,18 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons
 		emit_line(set, reset, line + i, len - i);
 }
 
+static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
+{
+	const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
+	const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW);
+
+	if (!*ws)
+		emit_line(set, reset, line, len);
+	else
+		emit_line_with_ws(ecbdata->nparents, set, reset, reset,
+				line, len);
+}
+
 static void fn_out_consume(void *priv, char *line, unsigned long len)
 {
 	int i;
@@ -864,30 +870,45 @@ static void show_numstat(struct diffstat_t* data, struct diff_options *options)
 struct checkdiff_t {
 	struct xdiff_emit_state xm;
 	const char *filename;
-	int lineno;
+	int lineno, color_diff;
 };
 
 static void checkdiff_consume(void *priv, char *line, unsigned long len)
 {
 	struct checkdiff_t *data = priv;
+	const char *ws = diff_get_color(data->color_diff, DIFF_WHITESPACE);
+	const char *reset = diff_get_color(data->color_diff, DIFF_RESET);
+	const char *set = diff_get_color(data->color_diff, DIFF_FILE_NEW);
 
 	if (line[0] == '+') {
-		int i, spaces = 0;
+		int i, spaces = 0, space_before_tab = 0, white_space_at_end = 0;
+		const char *message = NULL;
 
 		/* check space before tab */
 		for (i = 1; i < len && (line[i] == ' ' || line[i] == '\t'); i++)
 			if (line[i] == ' ')
 				spaces++;
 		if (line[i - 1] == '\t' && spaces)
-			printf("%s:%d: space before tab:%.*s\n",
-				data->filename, data->lineno, (int)len, line);
+			space_before_tab = 1;
 
 		/* check white space at line end */
 		if (line[len - 1] == '\n')
 			len--;
 		if (isspace(line[len - 1]))
-			printf("%s:%d: white space at end: %.*s\n",
-				data->filename, data->lineno, (int)len, line);
+			white_space_at_end = 1;
+
+		if (space_before_tab || white_space_at_end) {
+			printf("%s:%d: %s", data->filename, data->lineno, ws);
+			if (space_before_tab) {
+				printf("space before tab");
+				if (white_space_at_end)
+					putchar(',');
+			}
+			if (white_space_at_end)
+				printf("white space at end");
+			printf(":%s ", reset);
+			emit_line_with_ws(2, set, reset, ws, line, len);
+		}
 
 		data->lineno++;
 	} else if (line[0] == ' ')
@@ -1145,7 +1166,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
 
 static void builtin_checkdiff(const char *name_a, const char *name_b,
 			     struct diff_filespec *one,
-			     struct diff_filespec *two)
+			     struct diff_filespec *two, struct diff_options *o)
 {
 	mmfile_t mf1, mf2;
 	struct checkdiff_t data;
@@ -1157,6 +1178,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
 	data.xm.consume = checkdiff_consume;
 	data.filename = name_b ? name_b : name_a;
 	data.lineno = 0;
+	data.color_diff = o->color_diff;
 
 	if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 		die("unable to read files to diff");
@@ -1766,7 +1788,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
 	diff_fill_sha1_info(p->one);
 	diff_fill_sha1_info(p->two);
 
-	builtin_checkdiff(name, other, p->one, p->two);
+	builtin_checkdiff(name, other, p->one, p->two, o);
 }
 
 void diff_setup(struct diff_options *options)
-- 
1.5.0.rc2.g8b13f-dirty
Junio C Hamano· Jan 24, 2007, 23:18 UTC · re: Johannes Schindelin · lore

Re: [PATCH] diff --check: use colour

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Reuse the colour handling of the regular diff.
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 7 quoted lines
> Reuse the colour handling of the regular diff.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ---
>
> 	Is it possible that diff_get_color() _never_ returns NULL?
> 	Then diff.c can be further cleaned up...

Does anybody actually use "diff --check"? We could lose more code by removing the option altogether ;-)

Seriously, this option needs to be documented.  Volunteers?
Junio C Hamano· Jan 25, 2007, 22:23 UTC · re: Johannes Schindelin · lore

Re: [PATCH] diff --check: use colour

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 5 quoted lines
> On Wed, 24 Jan 2007, Junio C Hamano wrote:
>
>> Does anybody actually use "diff --check"?
>
> Yes, I do.
Then
	Seriously, this option needs to be documented.  Volunteers?
Bill Lear· Jan 25, 2007, 22:49 UTC · re: Junio C Hamano · lore

Re: [PATCH] diff --check: use colour

On Thursday, January 25, 2007 at 14:23:07 (-0800) Junio C Hamano writes:
>...
>Then
>
>	Seriously, this option needs to be documented.  Volunteers?

If you think someone new to git could do this, I would be happy to make this my first contribution to the git community.

Tell me where to look for the thing to modify, how to modify it, and how to tell you about what I did (etc.), and I'll give it a go.

Bill
Johannes Schindelin· Jan 25, 2007, 23:41 UTC · re: Bill Lear · lore

Re: [PATCH] diff --check: use colour

Hi,
On Thu, 25 Jan 2007, Bill Lear wrote:
Show 11 quoted lines
> On Thursday, January 25, 2007 at 14:23:07 (-0800) Junio C Hamano writes:
> >...
> >Then
> >
> >	Seriously, this option needs to be documented.  Volunteers?
> 
> If you think someone new to git could do this, I would be happy to make 
> this my first contribution to the git community.
> 
> Tell me where to look for the thing to modify, how to modify it, and how 
> to tell you about what I did (etc.), and I'll give it a go.

The --check option is handled in diff.c (you can find out by e.g. "git grep -e --check -- \*"). Thus, it should be documented in Documentation/diff-options.txt.

The easiest way to find out what it does is to execute:
	git log -S--check diff.c
This will lead you to v1.4.0-rc1~110.

The file format for Documentation/*.txt is easily imitated by looking at the context, and maybe eventually "make doc" (you'll need asciidoc for that).

Once you made that change, just commit and make a patch with "git format-patch HEAD^". Submit. Be famous.

Thank you, Dscho

Bill Lear· Jan 26, 2007, 00:06 UTC · re: Johannes Schindelin · lore

Re: [PATCH] diff --check: use colour

On Friday, January 26, 2007 at 00:41:42 (+0100) Johannes Schindelin writes:
>...
>The easiest way to find out what it does is to execute:
>
>	git log -S--check diff.c
Hmm, using 1.5.0-rc2, I created a test repo, and did this:

echo foo > foo git add foo git commit -a -m foo echo bar > foo git commit -a -m bar git log -S--check foo

and nothing happened.
I did
git log -S --check foo

and the thing went off into outer space. Now at over 2 1/2 minutes of CPU time on my 2 Ghz Opteron box...

Is it really '-S--check'?
Bill
Jeff King· Jan 26, 2007, 04:31 UTC · re: Bill Lear · lore

Re: [PATCH] diff --check: use colour

On Thu, Jan 25, 2007 at 06:06:51PM -0600, Bill Lear wrote:
> >The easiest way to find out what it does is to execute:
> >	git log -S--check diff.c
> 
> Hmm, using 1.5.0-rc2, I created a test repo, and did this:

I think you misunderstood Johannes. The -S option is used to find revisions that include the given string in their changes. So he was suggesting running that command in the _git_ repository, to show you the commit that introduced the --check option (from which you would get some material for writing the docs...)

Show 8 quoted lines
> echo foo > foo
> git add foo
> git commit -a -m foo
> echo bar > foo
> git commit -a -m bar
> git log -S--check foo
> 
> and nothing happened.
Right. You never put the word --check into your repository content. :)
> git log -S --check foo
> 
> and the thing went off into outer space.  Now at over 2 1/2 minutes of
> CPU time on my 2 Ghz Opteron box...
This appears to be a bug. I will look into it.
-Peff
Andreas Ericsson· Jan 26, 2007, 11:10 UTC · re: Bill Lear · lore

Re: [PATCH] diff --check: use colour

Bill Lear wrote:
Show 26 quoted lines
> On Friday, January 26, 2007 at 00:41:42 (+0100) Johannes Schindelin writes:
>> ...
>> The easiest way to find out what it does is to execute:
>>
>> 	git log -S--check diff.c
> 
> Hmm, using 1.5.0-rc2, I created a test repo, and did this:
> 
> echo foo > foo
> git add foo
> git commit -a -m foo
> echo bar > foo
> git commit -a -m bar
> git log -S--check foo
> 
> and nothing happened.
> 
> I did
> 
> git log -S --check foo
> 
> and the thing went off into outer space.  Now at over 2 1/2 minutes of
> CPU time on my 2 Ghz Opteron box...
> 
> Is it really '-S--check'?
> 

Yes. '-S--check' will start a pickaxe search for the string '--check' in the repository, covering all the history and the likes. It can take quite a long time (even if used properly ;-)), but it's certainly better than having to look manually.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Andy Parkins· Jan 26, 2007, 12:05 UTC · re: Bill Lear · lore

Re: [PATCH] diff --check: use colour

On Friday 2007 January 26 00:06, Bill Lear wrote:
> git log -S --check foo
>
> and the thing went off into outer space.  Now at over 2 1/2 minutes of
> CPU time on my 2 Ghz Opteron box...

I would guess it's because you've used "-S<space>--check"; the --check will be ignored as git-log doesn't use it, but the -S is saying "search for the empty string" in all log messages - that's probably a long list, and so takes a while to compile.

> Is it really '-S--check'?

Yep - it's "-S<string to search for>", so you'd be searching for the string "--check" in that example.

Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com
Bill Lear· Jan 26, 2007, 13:52 UTC · re: Andy Parkins · lore

Re: [PATCH] diff --check: use colour

On Friday, January 26, 2007 at 12:05:27 (+0000) Andy Parkins writes:
Show 11 quoted lines
>On Friday 2007 January 26 00:06, Bill Lear wrote:
>
>> git log -S --check foo
>>
>> and the thing went off into outer space.  Now at over 2 1/2 minutes of
>> CPU time on my 2 Ghz Opteron box...
>
>I would guess it's because you've used "-S<space>--check"; the --check will be 
>ignored as git-log doesn't use it, but the -S is saying "search for the empty 
>string" in all log messages - that's probably a long list, and so takes a 
>while to compile.

Well, remember this was a test repo, one file, one commit, with the word "foo" in the file. Even if searching for a space, it should not take that long. I think it's an infinite loop somewhere.

Bill
Bill Lear· Jan 26, 2007, 15:49 UTC · re: Johannes Schindelin · lore

Re: [PATCH] diff --check: use colour

On Friday, January 26, 2007 at 00:41:42 (+0100) Johannes Schindelin writes:
Show 7 quoted lines
>...
>The file format for Documentation/*.txt is easily imitated by looking at 
>the context, and maybe eventually "make doc" (you'll need asciidoc for 
>that).
>
>Once you made that change, just commit and make a patch with "git 
>format-patch HEAD^". Submit. Be famous.

Ok, I've read through the code and, have experimented with 'diff --check' fairly extensively, now understand it, have documented what I understand, and am now building the doc to ensure it is properly formatted.

I presume that I send the patch text to the list with the subject line that starts with [PATCH], some sensible topic (e.g., "document --check option to diff"), and then a short body of explanation and then, following my signature, the patch itself.

Assuming that is correct, I should have a patch out before the end of the day.

Bill
J. Bruce Fields· Jan 26, 2007, 16:00 UTC · re: Bill Lear · lore

Re: [PATCH] diff --check: use colour

On Fri, Jan 26, 2007 at 09:49:13AM -0600, Bill Lear wrote:
Show 8 quoted lines
> Ok, I've read through the code and, have experimented with 'diff --check'
> fairly extensively, now understand it, have documented what I understand,
> and am now building the doc to ensure it is properly formatted.
> 
> I presume that I send the patch text to the list with the subject line
> that starts with [PATCH], some sensible topic (e.g., "document --check
> option to diff"), and then a short body of explanation and then,
> following my signature, the patch itself.
See also Documentation/SubmittingPatches in the git tree.
--b.
Bill Lear· Jan 26, 2007, 16:07 UTC · re: J. Bruce Fields · lore

Re: [PATCH] diff --check: use colour

On Friday, January 26, 2007 at 11:00:40 (-0500) J. Bruce Fields writes:
>...
>See also Documentation/SubmittingPatches in the git tree.
Sorry, missed that before my previous post went out...
Bill
Bill Lear· Jan 26, 2007, 16:06 UTC · re: Bill Lear · lore

Re: [PATCH] diff --check: use colour

On Friday, January 26, 2007 at 09:49:13 (-0600) Bill Lear writes:
>...
>Assuming that is correct, I should have a patch out before the end of
>the day.
I did this:
git format-patch -s HEAD^
it produced this file:
0001-Document-check-option-to-git-diff.patch
which looks to be formatted as a mail message:
Show 12 quoted lines
>From 2a81f1e97564b89ab622cf32b68e7cebf605eafe Mon Sep 17 00:00:00 2001
>From: Bill Lear <rael@zopyra.com>
>Date: Fri, 26 Jan 2007 09:58:07 -0600
>Subject: [PATCH] Document --check option to git-diff.
>
>
>Signed-off-by: Bill Lear <rael@zopyra.com>
>---
> Documentation/diff-options.txt |    5 +++++
> Documentation/git-diff.txt     |    6 ++++++
> 2 files changed, 11 insertions(+), 0 deletions(-)
>...

Do I optionally edit this file, putting my comments before the "Signed-off-by" line, and then just send this off to the list with my mail command, i.e.:

% mail git@vger.kernel.org < 0001-Document-check-option-to-git-diff.patch
?
Bill
Jeff King· Jan 26, 2007, 17:19 UTC · re: Bill Lear · lore

Re: [PATCH] diff --check: use colour

On Fri, Jan 26, 2007 at 10:06:54AM -0600, Bill Lear wrote:
> git format-patch -s HEAD^
> [...]
> which looks to be formatted as a mail message:
Right.
The format is:
  - subject line is the first line of your commit message
  - subsequent body lines are the rest of your commit message, until you
    hit the ---
  - below that is the patch; everything in the patch section that is not
    an actual diff hunk is free-form (e.g., the diffstat).
> Do I optionally edit this file, putting my comments before the
> "Signed-off-by" line, and then just send this off to the list with my
> mail command, i.e.:

Comments before the signed-off-by line will be part of the commit message; so if you have comments to put there, you probably would have made them when your were committing. If you have comments to send with the email ("cover letter"), place them after the --- but before the diffstat.

> % mail git@vger.kernel.org < 0001-Document-check-option-to-git-diff.patch
> 
> ?

I'm not an expert on 'mail' but I'm not sure if it handles headers correctly (i.e., it expects body text, _not_ the full rfc822 message). You can use git-send-email, but I find its interface a little clunky. You can use git-imap-send to place the message in an IMAP mailbox, which you can then presumably grab as a 'draft' using your regular mail client. Or you can use a mail client which understands mbox (most unix ones do), open the file as an mbox, and then resume it as a draft.

Or finally, you can simply edit in the headers you want, remove the initial 'From ' line, and pipe it to sendmail. :)

What's easiest for you depends on what your mail setup is like (I use mutt to open the mbox, then "resend" it with editing).

-Peff
Jakub Narebski· Jan 26, 2007, 17:26 UTC · re: Bill Lear · lore

Re: [PATCH] diff --check: use colour

[Cc: git@vger.kernel.org]
Bill Lear wrote:
Show 32 quoted lines
> On Friday, January 26, 2007 at 09:49:13 (-0600) Bill Lear writes:
>>...
>>Assuming that is correct, I should have a patch out before the end of
>>the day.
> 
> I did this:
> 
> git format-patch -s HEAD^
> 
> it produced this file:
> 
> 0001-Document-check-option-to-git-diff.patch
> 
> which looks to be formatted as a mail message:
> 
>>From 2a81f1e97564b89ab622cf32b68e7cebf605eafe Mon Sep 17 00:00:00 2001
>>From: Bill Lear <rael@zopyra.com>
>>Date: Fri, 26 Jan 2007 09:58:07 -0600
>>Subject: [PATCH] Document --check option to git-diff.
>>
>>
>>Signed-off-by: Bill Lear <rael@zopyra.com>
>>---
>> Documentation/diff-options.txt |    5 +++++
>> Documentation/git-diff.txt     |    6 ++++++
>> 2 files changed, 11 insertions(+), 0 deletions(-)
>>...
> 
> 
> Do I optionally edit this file, putting my comments before the
> "Signed-off-by" line, and then just send this off to the list with my
> mail command, i.e.:

Before "Signed-off-by" line you put commit message. Any coments which are not meant to be in commit message (like for example reply to some email) are to be put after "---" line, but before diffstat.

Read Documentation/SubmittingPatches
> % mail git@vger.kernel.org < 0001-Document-check-option-to-git-diff.patch
This should work.
% git send-email --to git@vger.kernel.org 0001-Document-check-option-to-git-diff.patch
should also work.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Andy Parkins· Jan 26, 2007, 16:37 UTC · re: Bill Lear · lore

Re: [PATCH] diff --check: use colour

On Friday 2007 January 26 15:49, Bill Lear wrote:
> I presume that I send the patch text to the list with the subject line
> that starts with [PATCH], some sensible topic (e.g., "document --check
> option to diff"), and then a short body of explanation and then,
> following my signature, the patch itself.

git-format-patch will make you an email of the correct form. Personally I do this (this is the IMAP server version, but it's similar for mbox)

 * make myself a branch from current master
 * write patch/patches, test (yeah, right ;-))
 * git-format-patch --stdout HEAD^ to check it looks good
 * git-format-patch --stdout HEAD^ | git-imap-send 
 * Go to Drafts mailbox, open it up, add any additional comments for the
   mailing list only underneath the "---" and before the diffstat.  Make sure
   you turn word wrap off as soon as you open the mail.  Word wrapping will
   ruin the patch.
 * Send.
 * Wait for huge feeling of disappointment because your patch is junk and
   gets savaged by the git-gurus (maybe this step is just for me though).

To make this all easy and automatic I also have the following in my .git/config:

[format]
        headers = "To: git@vger.kernel.org\n"
And the following in my ~/.gitconfig:
[imap]
    Folder = "INBOX.Drafts"
    Tunnel = "ssh -q mailhost /usr/bin/imapd ./IMAPmd 2> /dev/null"

And the following in my .git/hooks/commit-msg (make sure you set it executable) to automatically add the "Signed-Off-By" line to your commits:

SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p') grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"

I've also enabled the .git/hooks/pre-commit hook (chmod a+x) to make git check my patches before they get committed.

git-format-patch makes git mailing list friendly patches that match the guidelines given in Documentation/SubmittingPatches

Hope that helps.
Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com
Jakub Narebski· Jan 26, 2007, 17:41 UTC · re: Andy Parkins · lore

Re: [PATCH] diff --check: use colour

Andy Parkins wrote:
Show 9 quoted lines
> On Friday 2007 January 26 15:49, Bill Lear wrote:
> 
>> I presume that I send the patch text to the list with the subject line
>> that starts with [PATCH], some sensible topic (e.g., "document --check
>> option to diff"), and then a short body of explanation and then,
>> following my signature, the patch itself.
> 
> git-format-patch will make you an email of the correct form.  Personally I do 
> this (this is the IMAP server version, but it's similar for mbox)

You can use git-send-email to send emails via SMTP. I have configured sendmail to send emails via gmail

-dnl define(`SMART_HOST',`smtp.your.provider')
+define(`SMART_HOST',`[smtp.gmail.com]')
 
>  * make myself a branch from current master
>  * write patch/patches, test (yeah, right ;-))

The same for me (with the exception that I branch from origin). If writing patches takes longer time, and upstream has advanced in meantime, I also rebase before sending patches.

   * rebase branch using "git rebase master <branch>"
Show 6 quoted lines
>  * git-format-patch --stdout HEAD^ to check it looks good
>  * git-format-patch --stdout HEAD^ | git-imap-send 
>  * Go to Drafts mailbox, open it up, add any additional comments for the
>    mailing list only underneath the "---" and before the diffstat.  Make sure
>    you turn word wrap off as soon as you open the mail.  Word wrapping will
>    ruin the patch.

For me it is "git format-patch HEAD^.." if it is one patch (one commit) only, "git format-patch -o mdir.<n> origin.." if this is series of patches. Then edit the patches (the 0001-*.txt file), adding any additional comments for the mailing list only underneath the "---" and before the diffstat.

>  * Send.
Use "git send-email <patch>" or "git send-email mdir.<n>" to send patches.
>  * Wait for huge feeling of disappointment because your patch is junk and
>    gets savaged by the git-gurus (maybe this step is just for me though).
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Junio C Hamano· Jan 26, 2007, 00:17 UTC · re: Johannes Schindelin · lore

Re: [PATCH] diff --check: use colour

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 	Is it possible that diff_get_color() _never_ returns NULL?

I think so, although it may often return a pointer that points at NUL (e.g under --no-color option).

Johannes Schindelin· Jan 26, 2007, 21:15 UTC · re: Junio C Hamano · lore

Re: [PATCH] diff --check: use colour

Hi,
On Thu, 25 Jan 2007, Junio C Hamano wrote:
Show 6 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 	Is it possible that diff_get_color() _never_ returns NULL?
> 
> I think so, although it may often return a pointer that points
> at NUL (e.g under --no-color option).

Ah! I misread that particular part of the code. It says "if (!*ws)", not -- as I read it -- "if (!ws)".

Ciao, Dscho

Johannes Schindelin· Feb 18, 2007, 00:47 UTC · re: Junio C Hamano · lore

Re: [PATCH] diff --check: use colour

Hi,
On Sat, 17 Feb 2007, Junio C Hamano wrote:
> I wonder what happened to this patch...

I think it is still the same. At least I did not use another patch in my private tree, and it works fine.

I _thought_ I could clean it up a little more, but then realized a thinko on my part: I misread a NUL for a NULL.

Should I resend?

Ciao, Dscho

Junio C Hamano· Feb 18, 2007, 01:06 UTC · re: Johannes Schindelin · lore

Re: [PATCH] diff --check: use colour

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> I _thought_ I could clean it up a little more, but then realized a thinko 
> on my part: I misread a NUL for a NULL.
>
> Should I resend?

No need. I think you have unused variable 'message' but I'll manage.

Johannes Schindelin· Feb 18, 2007, 01:15 UTC · re: Junio C Hamano · lore

Re: [PATCH] diff --check: use colour

Hi,
On Sat, 17 Feb 2007, Junio C Hamano wrote:
Show 9 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > I _thought_ I could clean it up a little more, but then realized a thinko 
> > on my part: I misread a NUL for a NULL.
> >
> > Should I resend?
> 
> No need.  I think you have unused variable 'message' but I'll
> manage.
Ah yes. And I'm sure you'll manage. ;-)

Ciao, Dscho

Johannes Schindelin· Feb 18, 2007, 16:27 UTC · re: Johannes Schindelin · lore

[PATCH for "master"] diff --check: use colour

Reuse the colour handling of the regular diff.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
	This time I tried to make sure that the patch applies cleanly
	to "master", that there are no compiler warnings, and that no
	behaviour (apart from --check) is changed.
 diff.c |   57 +++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 39 insertions(+), 18 deletions(-)
Show changes to diff.c +39 −18
diff --git a/diff.c b/diff.c
index 701880a..ce25f4b 100644
--- a/diff.c
+++ b/diff.c
@@ -405,22 +405,16 @@ static void emit_line(const char *set, const char *reset, const char *line, int
 	puts(reset);
 }
 
-static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
+static void emit_line_with_ws(int nparents,
+		const char *set, const char *reset, const char *ws,
+		const char *line, int len)
 {
-	int col0 = ecbdata->nparents;
+	int col0 = nparents;
 	int last_tab_in_indent = -1;
 	int last_space_in_indent = -1;
 	int i;
 	int tail = len;
 	int need_highlight_leading_space = 0;
-	const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
-	const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW);
-
-	if (!*ws) {
-		emit_line(set, reset, line, len);
-		return;
-	}
-
 	/* The line is a newly added line.  Does it have funny leading
 	 * whitespaces?  In indent, SP should never precede a TAB.
 	 */
@@ -475,6 +469,18 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons
 		emit_line(set, reset, line + i, len - i);
 }
 
+static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
+{
+	const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
+	const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW);
+
+	if (!*ws)
+		emit_line(set, reset, line, len);
+	else
+		emit_line_with_ws(ecbdata->nparents, set, reset, ws,
+				line, len);
+}
+
 static void fn_out_consume(void *priv, char *line, unsigned long len)
 {
 	int i;
@@ -884,30 +890,44 @@ static void show_numstat(struct diffstat_t* data, struct diff_options *options)
 struct checkdiff_t {
 	struct xdiff_emit_state xm;
 	const char *filename;
-	int lineno;
+	int lineno, color_diff;
 };
 
 static void checkdiff_consume(void *priv, char *line, unsigned long len)
 {
 	struct checkdiff_t *data = priv;
+	const char *ws = diff_get_color(data->color_diff, DIFF_WHITESPACE);
+	const char *reset = diff_get_color(data->color_diff, DIFF_RESET);
+	const char *set = diff_get_color(data->color_diff, DIFF_FILE_NEW);
 
 	if (line[0] == '+') {
-		int i, spaces = 0;
+		int i, spaces = 0, space_before_tab = 0, white_space_at_end = 0;
 
 		/* check space before tab */
 		for (i = 1; i < len && (line[i] == ' ' || line[i] == '\t'); i++)
 			if (line[i] == ' ')
 				spaces++;
 		if (line[i - 1] == '\t' && spaces)
-			printf("%s:%d: space before tab:%.*s\n",
-				data->filename, data->lineno, (int)len, line);
+			space_before_tab = 1;
 
 		/* check white space at line end */
 		if (line[len - 1] == '\n')
 			len--;
 		if (isspace(line[len - 1]))
-			printf("%s:%d: white space at end: %.*s\n",
-				data->filename, data->lineno, (int)len, line);
+			white_space_at_end = 1;
+
+		if (space_before_tab || white_space_at_end) {
+			printf("%s:%d: %s", data->filename, data->lineno, ws);
+			if (space_before_tab) {
+				printf("space before tab");
+				if (white_space_at_end)
+					putchar(',');
+			}
+			if (white_space_at_end)
+				printf("white space at end");
+			printf(":%s ", reset);
+			emit_line_with_ws(1, set, reset, ws, line, len);
+		}
 
 		data->lineno++;
 	} else if (line[0] == ' ')
@@ -1165,7 +1185,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
 
 static void builtin_checkdiff(const char *name_a, const char *name_b,
 			     struct diff_filespec *one,
-			     struct diff_filespec *two)
+			     struct diff_filespec *two, struct diff_options *o)
 {
 	mmfile_t mf1, mf2;
 	struct checkdiff_t data;
@@ -1177,6 +1197,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
 	data.xm.consume = checkdiff_consume;
 	data.filename = name_b ? name_b : name_a;
 	data.lineno = 0;
+	data.color_diff = o->color_diff;
 
 	if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 		die("unable to read files to diff");
@@ -1787,7 +1808,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
 	diff_fill_sha1_info(p->one);
 	diff_fill_sha1_info(p->two);
 
-	builtin_checkdiff(name, other, p->one, p->two);
+	builtin_checkdiff(name, other, p->one, p->two, o);
 }
 
 void diff_setup(struct diff_options *options)
-- 
1.5.0.52.gf6b7-dirty

← back to recent threads