All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] diff --check: use colour
@ 2007-01-24 14:05 Johannes Schindelin
  2007-01-24 23:18 ` Junio C Hamano
  2007-01-26  0:17 ` Junio C Hamano
  0 siblings, 2 replies; 26+ messages in thread
From: Johannes Schindelin @ 2007-01-24 14:05 UTC (permalink / raw)
  To: git, junkio


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(-)

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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-24 14:05 [PATCH] diff --check: use colour Johannes Schindelin
@ 2007-01-24 23:18 ` Junio C Hamano
  2007-01-25  9:16   ` Johannes Schindelin
  2007-01-26  0:17 ` Junio C Hamano
  1 sibling, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2007-01-24 23:18 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, junkio

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Reuse the colour handling of the regular diff.

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 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?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-24 23:18 ` Junio C Hamano
@ 2007-01-25  9:16   ` Johannes Schindelin
  2007-01-25 22:23     ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Johannes Schindelin @ 2007-01-25  9:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi,

On Wed, 24 Jan 2007, Junio C Hamano wrote:

> Does anybody actually use "diff --check"?

Yes, I do.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-25  9:16   ` Johannes Schindelin
@ 2007-01-25 22:23     ` Junio C Hamano
  2007-01-25 22:49       ` Bill Lear
  0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2007-01-25 22:23 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 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?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-25 22:23     ` Junio C Hamano
@ 2007-01-25 22:49       ` Bill Lear
  2007-01-25 23:41         ` Johannes Schindelin
  0 siblings, 1 reply; 26+ messages in thread
From: Bill Lear @ 2007-01-25 22:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-25 22:49       ` Bill Lear
@ 2007-01-25 23:41         ` Johannes Schindelin
  2007-01-26  0:06           ` Bill Lear
  2007-01-26 15:49           ` Bill Lear
  0 siblings, 2 replies; 26+ messages in thread
From: Johannes Schindelin @ 2007-01-25 23:41 UTC (permalink / raw)
  To: Bill Lear; +Cc: Junio C Hamano, git

Hi,

On Thu, 25 Jan 2007, Bill Lear wrote:

> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-25 23:41         ` Johannes Schindelin
@ 2007-01-26  0:06           ` Bill Lear
  2007-01-26  4:31             ` Jeff King
                               ` (2 more replies)
  2007-01-26 15:49           ` Bill Lear
  1 sibling, 3 replies; 26+ messages in thread
From: Bill Lear @ 2007-01-26  0:06 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-24 14:05 [PATCH] diff --check: use colour Johannes Schindelin
  2007-01-24 23:18 ` Junio C Hamano
@ 2007-01-26  0:17 ` Junio C Hamano
  2007-01-26 21:15   ` Johannes Schindelin
  1 sibling, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2007-01-26  0:17 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

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).

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26  0:06           ` Bill Lear
@ 2007-01-26  4:31             ` Jeff King
  2007-01-26 11:10             ` Andreas Ericsson
  2007-01-26 12:05             ` Andy Parkins
  2 siblings, 0 replies; 26+ messages in thread
From: Jeff King @ 2007-01-26  4:31 UTC (permalink / raw)
  To: Bill Lear; +Cc: Johannes Schindelin, Junio C Hamano, git

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...)

> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26  0:06           ` Bill Lear
  2007-01-26  4:31             ` Jeff King
@ 2007-01-26 11:10             ` Andreas Ericsson
  2007-01-26 12:05             ` Andy Parkins
  2 siblings, 0 replies; 26+ messages in thread
From: Andreas Ericsson @ 2007-01-26 11:10 UTC (permalink / raw)
  To: Bill Lear; +Cc: Johannes Schindelin, Junio C Hamano, git

Bill Lear wrote:
> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26  0:06           ` Bill Lear
  2007-01-26  4:31             ` Jeff King
  2007-01-26 11:10             ` Andreas Ericsson
@ 2007-01-26 12:05             ` Andy Parkins
  2007-01-26 13:52               ` Bill Lear
  2 siblings, 1 reply; 26+ messages in thread
From: Andy Parkins @ 2007-01-26 12:05 UTC (permalink / raw)
  To: git; +Cc: Bill Lear, Johannes Schindelin, Junio C Hamano

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26 12:05             ` Andy Parkins
@ 2007-01-26 13:52               ` Bill Lear
  0 siblings, 0 replies; 26+ messages in thread
From: Bill Lear @ 2007-01-26 13:52 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git, Johannes Schindelin, Junio C Hamano

On Friday, January 26, 2007 at 12:05:27 (+0000) Andy Parkins writes:
>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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-25 23:41         ` Johannes Schindelin
  2007-01-26  0:06           ` Bill Lear
@ 2007-01-26 15:49           ` Bill Lear
  2007-01-26 16:00             ` J. Bruce Fields
                               ` (2 more replies)
  1 sibling, 3 replies; 26+ messages in thread
From: Bill Lear @ 2007-01-26 15:49 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git

On Friday, January 26, 2007 at 00:41:42 (+0100) Johannes Schindelin writes:
>...
>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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26 15:49           ` Bill Lear
@ 2007-01-26 16:00             ` J. Bruce Fields
  2007-01-26 16:07               ` Bill Lear
  2007-01-26 16:06             ` Bill Lear
  2007-01-26 16:37             ` Andy Parkins
  2 siblings, 1 reply; 26+ messages in thread
From: J. Bruce Fields @ 2007-01-26 16:00 UTC (permalink / raw)
  To: Bill Lear; +Cc: Johannes Schindelin, Junio C Hamano, git

On Fri, Jan 26, 2007 at 09:49:13AM -0600, Bill Lear wrote:
> 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.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26 15:49           ` Bill Lear
  2007-01-26 16:00             ` J. Bruce Fields
@ 2007-01-26 16:06             ` Bill Lear
  2007-01-26 17:19               ` Jeff King
  2007-01-26 17:26               ` Jakub Narebski
  2007-01-26 16:37             ` Andy Parkins
  2 siblings, 2 replies; 26+ messages in thread
From: Bill Lear @ 2007-01-26 16:06 UTC (permalink / raw)
  To: Johannes Schindelin, Junio C Hamano, git

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.:

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

?


Bill

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26 16:00             ` J. Bruce Fields
@ 2007-01-26 16:07               ` Bill Lear
  0 siblings, 0 replies; 26+ messages in thread
From: Bill Lear @ 2007-01-26 16:07 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Johannes Schindelin, Junio C Hamano, git

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26 15:49           ` Bill Lear
  2007-01-26 16:00             ` J. Bruce Fields
  2007-01-26 16:06             ` Bill Lear
@ 2007-01-26 16:37             ` Andy Parkins
  2007-01-26 17:41               ` Jakub Narebski
  2 siblings, 1 reply; 26+ messages in thread
From: Andy Parkins @ 2007-01-26 16:37 UTC (permalink / raw)
  To: git; +Cc: Bill Lear

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26 16:06             ` Bill Lear
@ 2007-01-26 17:19               ` Jeff King
  2007-01-26 17:26               ` Jakub Narebski
  1 sibling, 0 replies; 26+ messages in thread
From: Jeff King @ 2007-01-26 17:19 UTC (permalink / raw)
  To: Bill Lear; +Cc: git

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26 16:06             ` Bill Lear
  2007-01-26 17:19               ` Jeff King
@ 2007-01-26 17:26               ` Jakub Narebski
  1 sibling, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2007-01-26 17:26 UTC (permalink / raw)
  To: git

[Cc: git@vger.kernel.org]

Bill Lear wrote:

> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26 16:37             ` Andy Parkins
@ 2007-01-26 17:41               ` Jakub Narebski
  0 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2007-01-26 17:41 UTC (permalink / raw)
  To: git

Andy Parkins wrote:

> 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>"

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26  0:17 ` Junio C Hamano
@ 2007-01-26 21:15   ` Johannes Schindelin
  2007-02-18  0:25     ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Johannes Schindelin @ 2007-01-26 21:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi,

On Thu, 25 Jan 2007, Junio C Hamano wrote:

> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-01-26 21:15   ` Johannes Schindelin
@ 2007-02-18  0:25     ` Junio C Hamano
  2007-02-18  0:47       ` Johannes Schindelin
  0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2007-02-18  0:25 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

I wonder what happened to this patch...

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-02-18  0:25     ` Junio C Hamano
@ 2007-02-18  0:47       ` Johannes Schindelin
  2007-02-18  1:06         ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Johannes Schindelin @ 2007-02-18  0:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-02-18  0:47       ` Johannes Schindelin
@ 2007-02-18  1:06         ` Junio C Hamano
  2007-02-18  1:15           ` Johannes Schindelin
  0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2007-02-18  1:06 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

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.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] diff --check: use colour
  2007-02-18  1:06         ` Junio C Hamano
@ 2007-02-18  1:15           ` Johannes Schindelin
  2007-02-18 16:27             ` [PATCH for "master"] " Johannes Schindelin
  0 siblings, 1 reply; 26+ messages in thread
From: Johannes Schindelin @ 2007-02-18  1:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi,

On Sat, 17 Feb 2007, Junio C Hamano wrote:

> 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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH for "master"] diff --check: use colour
  2007-02-18  1:15           ` Johannes Schindelin
@ 2007-02-18 16:27             ` Johannes Schindelin
  0 siblings, 0 replies; 26+ messages in thread
From: Johannes Schindelin @ 2007-02-18 16:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git


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(-)

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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2007-02-18 16:27 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-01-24 14:05 [PATCH] diff --check: use colour Johannes Schindelin
2007-01-24 23:18 ` Junio C Hamano
2007-01-25  9:16   ` Johannes Schindelin
2007-01-25 22:23     ` Junio C Hamano
2007-01-25 22:49       ` Bill Lear
2007-01-25 23:41         ` Johannes Schindelin
2007-01-26  0:06           ` Bill Lear
2007-01-26  4:31             ` Jeff King
2007-01-26 11:10             ` Andreas Ericsson
2007-01-26 12:05             ` Andy Parkins
2007-01-26 13:52               ` Bill Lear
2007-01-26 15:49           ` Bill Lear
2007-01-26 16:00             ` J. Bruce Fields
2007-01-26 16:07               ` Bill Lear
2007-01-26 16:06             ` Bill Lear
2007-01-26 17:19               ` Jeff King
2007-01-26 17:26               ` Jakub Narebski
2007-01-26 16:37             ` Andy Parkins
2007-01-26 17:41               ` Jakub Narebski
2007-01-26  0:17 ` Junio C Hamano
2007-01-26 21:15   ` Johannes Schindelin
2007-02-18  0:25     ` Junio C Hamano
2007-02-18  0:47       ` Johannes Schindelin
2007-02-18  1:06         ` Junio C Hamano
2007-02-18  1:15           ` Johannes Schindelin
2007-02-18 16:27             ` [PATCH for "master"] " Johannes Schindelin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.