All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] t/t7502: compare entire commit message with what was expected
@ 2013-02-19  4:17 Brandon Casey
  2013-02-19  4:17 ` [PATCH 2/4] t7502: demonstrate breakage with a commit message with trailing newlines Brandon Casey
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Brandon Casey @ 2013-02-19  4:17 UTC (permalink / raw)
  To: git; +Cc: Brandon Casey

This test attempts to verify that a commit in "verbatim" mode, when
supplied a commit template, produces a commit in which the commit
message matches exactly the template that was supplied.  But, since the
commit operation appends additional instructions for the user as
comments in the commit buffer, which would cause the comparison to fail,
this test decided to compare only the first three lines (the length of
the template) of the resulting commit message to the original template
file.

This has two problems.

  1. It does not allow the template to be lengthened or shortened
     without also modifying the number of lines that are considered
     significant (i.e. the argument to 'head -n').
  2. It will not catch a bug in git that causes git to append additional
     lines to the commit message.

So, let's use the --no-status option to 'git commit' which will cause
git to refrain from appending the lines of instructional text to the
commit message.  This will allow the entire resulting commit message to
be compared against the expected value.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 t/t7502-commit.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index cbd7a45..9040f8a 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -181,8 +181,8 @@ test_expect_success 'cleanup commit messages (verbatim option,-t)' '
 
 	echo >>negative &&
 	{ echo;echo "# text";echo; } >expect &&
-	git commit --cleanup=verbatim -t expect -a &&
-	git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
+	git commit --cleanup=verbatim --no-status -t expect -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d" >actual &&
 	test_cmp expect actual
 
 '
-- 
1.8.1.3.638.g372f416.dirty

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

* [PATCH 2/4] t7502: demonstrate breakage with a commit message with trailing newlines
  2013-02-19  4:17 [PATCH 1/4] t/t7502: compare entire commit message with what was expected Brandon Casey
@ 2013-02-19  4:17 ` Brandon Casey
  2013-02-19  5:39   ` Jonathan Nieder
  2013-02-19  4:17 ` [PATCH 3/4] git-commit: only append a newline to -m mesg if necessary Brandon Casey
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Brandon Casey @ 2013-02-19  4:17 UTC (permalink / raw)
  To: git; +Cc: Brandon Casey

This test attempts to verify that a commit message supplied to 'git
commit' via the -m switch was used in full as the commit message for a
commit when --cleanup=verbatim was used.

But, this test has been broken since it was introduced.  Since the
commit message containing trailing newlines was supplied to 'git commit'
using a command substitution, the trailing newlines were removed by the
shell.  This means that a string without any trailing newlines was
actually supplied to 'git commit'.

The test was able to complete successfully since internally, git appends
two newlines to each string supplied via the -m switch.  So, the two
newlines removed by the shell were then re-added by git, and the
resulting commit matched what was expected.

So, let's move the initial creation of the commit message string out
from within a previous test so that it stands alone.  Assign the desired
commit message to a variable using literal newlines.  Then populate the
expect file from the contents of the commit message variable.  This way
the shell variable becomes the authoritative source of the commit
message and can be supplied via the -m switch with the trailing newlines
intact.

Mark this test as failing, since it is not handled correctly by git.
As described above, git appends two extra newlines to every string
supplied via -m.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 t/t7502-commit.sh | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 9040f8a..39e55f8 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -177,10 +177,18 @@ test_expect_success 'verbose respects diff config' '
 	git config --unset color.diff
 '
 
+mesg_with_comment_and_newlines='
+# text
+
+'
+
+test_expect_success 'prepare file with comment line and trailing newlines'  '
+	printf "%s" "$mesg_with_comment_and_newlines" >expect
+'
+
 test_expect_success 'cleanup commit messages (verbatim option,-t)' '
 
 	echo >>negative &&
-	{ echo;echo "# text";echo; } >expect &&
 	git commit --cleanup=verbatim --no-status -t expect -a &&
 	git cat-file -p HEAD |sed -e "1,/^\$/d" >actual &&
 	test_cmp expect actual
@@ -196,10 +204,10 @@ test_expect_success 'cleanup commit messages (verbatim option,-F)' '
 
 '
 
-test_expect_success 'cleanup commit messages (verbatim option,-m)' '
+test_expect_failure 'cleanup commit messages (verbatim option,-m)' '
 
 	echo >>negative &&
-	git commit --cleanup=verbatim -m "$(cat expect)" -a &&
+	git commit --cleanup=verbatim -m "$mesg_with_comment_and_newlines" -a &&
 	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
 	test_cmp expect actual
 
-- 
1.8.1.3.638.g372f416.dirty

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

* [PATCH 3/4] git-commit: only append a newline to -m mesg if necessary
  2013-02-19  4:17 [PATCH 1/4] t/t7502: compare entire commit message with what was expected Brandon Casey
  2013-02-19  4:17 ` [PATCH 2/4] t7502: demonstrate breakage with a commit message with trailing newlines Brandon Casey
@ 2013-02-19  4:17 ` Brandon Casey
  2013-02-19  6:31   ` Jonathan Nieder
  2013-02-19  4:17 ` [PATCH 4/4] Documentation/git-commit.txt: correct a few minor grammatical mistakes Brandon Casey
  2013-02-19  5:08 ` [PATCH 1/4] t/t7502: compare entire commit message with what was expected Jonathan Nieder
  3 siblings, 1 reply; 19+ messages in thread
From: Brandon Casey @ 2013-02-19  4:17 UTC (permalink / raw)
  To: git; +Cc: Brandon Casey

Currently, git will append two newlines to every message supplied via
the -m switch.  The purpose of this is to allow -m to be supplied
multiple times and have each supplied string become a paragraph in the
resulting commit message.

Normally, this does not cause a problem since any trailing newlines will
be removed by the cleanup operation.  If cleanup=verbatim for example,
then the trailing newlines will not be removed and will survive into the
resulting commit message.

Instead, let's ensure that the string supplied to -m is newline terminated,
but only append a second newline when appending additional messages.

Fixes the test in t7502.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 builtin/commit.c  | 4 +++-
 t/t7502-commit.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 3348aa1..d21d07a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -124,8 +124,10 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 	if (unset)
 		strbuf_setlen(buf, 0);
 	else {
+		if (buf->len)
+			strbuf_addch(buf, '\n');
 		strbuf_addstr(buf, arg);
-		strbuf_addstr(buf, "\n\n");
+		strbuf_complete_line(buf);
 	}
 	return 0;
 }
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 39e55f8..292bc08 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -204,7 +204,7 @@ test_expect_success 'cleanup commit messages (verbatim option,-F)' '
 
 '
 
-test_expect_failure 'cleanup commit messages (verbatim option,-m)' '
+test_expect_success 'cleanup commit messages (verbatim option,-m)' '
 
 	echo >>negative &&
 	git commit --cleanup=verbatim -m "$mesg_with_comment_and_newlines" -a &&
-- 
1.8.1.3.638.g372f416.dirty

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

* [PATCH 4/4] Documentation/git-commit.txt: correct a few minor grammatical mistakes
  2013-02-19  4:17 [PATCH 1/4] t/t7502: compare entire commit message with what was expected Brandon Casey
  2013-02-19  4:17 ` [PATCH 2/4] t7502: demonstrate breakage with a commit message with trailing newlines Brandon Casey
  2013-02-19  4:17 ` [PATCH 3/4] git-commit: only append a newline to -m mesg if necessary Brandon Casey
@ 2013-02-19  4:17 ` Brandon Casey
  2013-02-19  6:43   ` Jonathan Nieder
  2013-02-19  5:08 ` [PATCH 1/4] t/t7502: compare entire commit message with what was expected Jonathan Nieder
  3 siblings, 1 reply; 19+ messages in thread
From: Brandon Casey @ 2013-02-19  4:17 UTC (permalink / raw)
  To: git; +Cc: Brandon Casey

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 Documentation/git-commit.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0eb79cc..8ae7619 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -174,10 +174,10 @@ OPTIONS
 --cleanup=<mode>::
 	This option sets how the commit message is cleaned up.
 	The  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
-	and 'default'. The 'default' mode will strip leading and
+	or 'default'. The 'default' mode will strip leading and
 	trailing empty lines and #commentary from the commit message
-	only if the message is to be edited. Otherwise only whitespace
-	removed. The 'verbatim' mode does not change message at all,
+	only if the message is to be edited. Otherwise only whitespace is
+	removed. The 'verbatim' mode does not change the message at all,
 	'whitespace' removes just leading/trailing whitespace lines
 	and 'strip' removes both whitespace and commentary. The default
 	can be changed by the 'commit.cleanup' configuration variable
-- 
1.8.1.3.638.g372f416.dirty

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

* Re: [PATCH 1/4] t/t7502: compare entire commit message with what was expected
  2013-02-19  4:17 [PATCH 1/4] t/t7502: compare entire commit message with what was expected Brandon Casey
                   ` (2 preceding siblings ...)
  2013-02-19  4:17 ` [PATCH 4/4] Documentation/git-commit.txt: correct a few minor grammatical mistakes Brandon Casey
@ 2013-02-19  5:08 ` Jonathan Nieder
  2013-02-19  5:10   ` Jonathan Nieder
  2013-02-19 17:24   ` Junio C Hamano
  3 siblings, 2 replies; 19+ messages in thread
From: Jonathan Nieder @ 2013-02-19  5:08 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git

Brandon Casey wrote:

> So, let's use the --no-status option to 'git commit' which will cause
> git to refrain from appending the lines of instructional text to the
> commit message.  This will allow the entire resulting commit message to
> be compared against the expected value.

The downside (not a new problem, but a downside nonetheless) is that
it means the test doesn't demonstrate what --cleanup=verbatim --status
will do.

How about something like this?

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

diff --git i/t/t7502-commit.sh w/t/t7502-commit.sh
index cbd7a459..64162fce 100755
--- i/t/t7502-commit.sh
+++ w/t/t7502-commit.sh
@@ -180,15 +180,37 @@ test_expect_success 'verbose respects diff config' '
 test_expect_success 'cleanup commit messages (verbatim option,-t)' '
 
 	echo >>negative &&
-	{ echo;echo "# text";echo; } >expect &&
-	git commit --cleanup=verbatim -t expect -a &&
-	git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
+	{
+		echo &&
+		echo "# text" &&
+		echo
+	} >template &&
+	{
+		cat template &&
+		cat <<-\EOF &&
+
+		# Please enter the commit message for your changes. Lines starting
+		# with '\''#'\'' will be kept; you may remove them yourself if you want to.
+		# An empty message aborts the commit.
+		#
+		# Author:    A U Thor <author@example.com>
+		#
+		EOF
+		git commit -a --dry-run
+	} >expect &&
+	git commit --cleanup=verbatim -t template -a &&
+	git cat-file -p HEAD |sed -e "1,/^\$/d" >actual &&
 	test_cmp expect actual
 
 '
 
 test_expect_success 'cleanup commit messages (verbatim option,-F)' '
 
+	{
+		echo &&
+		echo "# text" &&
+		echo
+	} >expect &&
 	echo >>negative &&
 	git commit --cleanup=verbatim -F expect -a &&
 	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&

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

* Re: [PATCH 1/4] t/t7502: compare entire commit message with what was expected
  2013-02-19  5:08 ` [PATCH 1/4] t/t7502: compare entire commit message with what was expected Jonathan Nieder
@ 2013-02-19  5:10   ` Jonathan Nieder
  2013-02-19 17:24   ` Junio C Hamano
  1 sibling, 0 replies; 19+ messages in thread
From: Jonathan Nieder @ 2013-02-19  5:10 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git

Jonathan Nieder wrote:

> +++ w/t/t7502-commit.sh
[...]
> +		# Please enter the commit message for your changes. Lines starting
> +		# with '\''#'\'' will be kept; you may remove them yourself if you want to.
> +		# An empty message aborts the commit.
> +		#
> +		# Author:    A U Thor <author@example.com>
> +		#
> +		EOF
> +		git commit -a --dry-run
> +	} >expect &&
> +	git commit --cleanup=verbatim -t template -a &&
> -	git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
> +	git cat-file -p HEAD |sed -e "1,/^\$/d" >actual &&
>  	test_cmp expect actual

Quick correction: this would use test_i18ncmp instead of test_cmp if
it ends up being a good idea.

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

* Re: [PATCH 2/4] t7502: demonstrate breakage with a commit message with trailing newlines
  2013-02-19  4:17 ` [PATCH 2/4] t7502: demonstrate breakage with a commit message with trailing newlines Brandon Casey
@ 2013-02-19  5:39   ` Jonathan Nieder
  0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Nieder @ 2013-02-19  5:39 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git, Alex Riesen, Johannes Schindelin

Brandon Casey wrote:

> This test attempts to verify that a commit message supplied to 'git
> commit' via the -m switch was used in full as the commit message for a
> commit when --cleanup=verbatim was used.
[...]
> The test was able to complete successfully since internally, git appends
> two newlines to each string supplied via the -m switch.
[...]
> Mark this test as failing, since it is not handled correctly by git.
> As described above, git appends two extra newlines to every string
> supplied via -m.

Good catch.  This is an old one, triggered by a combination of

 v1.5.4-rc0~78^2~23 builtin-commit: resurrect behavior for multiple -m
                    options, 2007-11-11

and

 v1.5.4-rc2~3^2 Allow selection of different cleanup modes for commit
                messages, 2007-12-22

The patch makes sense and makes the test easier to read, so
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

(Patch left unsnipped for reference.)

> Signed-off-by: Brandon Casey <drafnel@gmail.com>
> ---
>  t/t7502-commit.sh | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
> index 9040f8a..39e55f8 100755
> --- a/t/t7502-commit.sh
> +++ b/t/t7502-commit.sh
> @@ -177,10 +177,18 @@ test_expect_success 'verbose respects diff config' '
>  	git config --unset color.diff
>  '
>  
> +mesg_with_comment_and_newlines='
> +# text
> +
> +'
> +
> +test_expect_success 'prepare file with comment line and trailing newlines'  '
> +	printf "%s" "$mesg_with_comment_and_newlines" >expect
> +'
> +
>  test_expect_success 'cleanup commit messages (verbatim option,-t)' '
>  
>  	echo >>negative &&
> -	{ echo;echo "# text";echo; } >expect &&
>  	git commit --cleanup=verbatim --no-status -t expect -a &&
>  	git cat-file -p HEAD |sed -e "1,/^\$/d" >actual &&
>  	test_cmp expect actual
> @@ -196,10 +204,10 @@ test_expect_success 'cleanup commit messages (verbatim option,-F)' '
>  
>  '
>  
> -test_expect_success 'cleanup commit messages (verbatim option,-m)' '
> +test_expect_failure 'cleanup commit messages (verbatim option,-m)' '
>  
>  	echo >>negative &&
> -	git commit --cleanup=verbatim -m "$(cat expect)" -a &&
> +	git commit --cleanup=verbatim -m "$mesg_with_comment_and_newlines" -a &&
>  	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
>  	test_cmp expect actual
>  
> -- 

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

* Re: [PATCH 3/4] git-commit: only append a newline to -m mesg if necessary
  2013-02-19  4:17 ` [PATCH 3/4] git-commit: only append a newline to -m mesg if necessary Brandon Casey
@ 2013-02-19  6:31   ` Jonathan Nieder
  0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Nieder @ 2013-02-19  6:31 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git, Alex Riesen, Johannes Schindelin

Brandon Casey wrote:

> Currently, git will append two newlines to every message supplied via
> the -m switch.  The purpose of this is to allow -m to be supplied
> multiple times and have each supplied string become a paragraph in the
> resulting commit message.
>
> Normally, this does not cause a problem since any trailing newlines will
> be removed by the cleanup operation.  If cleanup=verbatim for example,
> then the trailing newlines will not be removed and will survive into the
> resulting commit message.
>
> Instead, let's ensure that the string supplied to -m is newline terminated,
> but only append a second newline when appending additional messages.
[...]
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -124,8 +124,10 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset)
>  	if (unset)
>  		strbuf_setlen(buf, 0);
>  	else {
> +		if (buf->len)
> +			strbuf_addch(buf, '\n');
>  		strbuf_addstr(buf, arg);
> -		strbuf_addstr(buf, "\n\n");
> +		strbuf_complete_line(buf);

As long as 'message' always consists of complete lines, this will
append 'arg' as a new paragraph, as desired.  And no other code path
touches 'message', so it always consists of complete lines.

Thanks for a clear patch and explanation.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

(rest of patch kept unsnipped for reference)

>  	}
>  	return 0;
>  }
> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
> index 39e55f8..292bc08 100755
> --- a/t/t7502-commit.sh
> +++ b/t/t7502-commit.sh
> @@ -204,7 +204,7 @@ test_expect_success 'cleanup commit messages (verbatim option,-F)' '
>  
>  '
>  
> -test_expect_failure 'cleanup commit messages (verbatim option,-m)' '
> +test_expect_success 'cleanup commit messages (verbatim option,-m)' '
>  
>  	echo >>negative &&
>  	git commit --cleanup=verbatim -m "$mesg_with_comment_and_newlines" -a &&
> -- 

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

* Re: [PATCH 4/4] Documentation/git-commit.txt: correct a few minor grammatical mistakes
  2013-02-19  4:17 ` [PATCH 4/4] Documentation/git-commit.txt: correct a few minor grammatical mistakes Brandon Casey
@ 2013-02-19  6:43   ` Jonathan Nieder
  2013-02-19  7:18     ` Brandon Casey
  0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Nieder @ 2013-02-19  6:43 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git, Ralf Thielow

Brandon Casey wrote:

> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -174,10 +174,10 @@ OPTIONS
>  --cleanup=<mode>::
>  	This option sets how the commit message is cleaned up.
>  	The  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
> -	and 'default'. The 'default' mode will strip leading and
> +	or 'default'. The 'default' mode will strip leading and
>  	trailing empty lines and #commentary from the commit message
> -	only if the message is to be edited. Otherwise only whitespace
> -	removed. The 'verbatim' mode does not change message at all,
> +	only if the message is to be edited. Otherwise only whitespace is
> +	removed. The 'verbatim' mode does not change the message at all,
>  	'whitespace' removes just leading/trailing whitespace lines
>  	and 'strip' removes both whitespace and commentary. The default
>  	can be changed by the 'commit.cleanup' configuration variable

Yeah, the current text is a bit choppy.  How about this?

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

--- i/Documentation/git-commit.txt
+++ w/Documentation/git-commit.txt
@@ -172,16 +172,25 @@ OPTIONS
        linkgit:git-commit-tree[1].
 
 --cleanup=<mode>::
-	This option sets how the commit message is cleaned up.
-	The  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
-	and 'default'. The 'default' mode will strip leading and
-	trailing empty lines and #commentary from the commit message
-	only if the message is to be edited. Otherwise only whitespace
-	removed. The 'verbatim' mode does not change message at all,
-	'whitespace' removes just leading/trailing whitespace lines
-	and 'strip' removes both whitespace and commentary. The default
-	can be changed by the 'commit.cleanup' configuration variable
-	(see linkgit:git-config[1]).
+	This option determines how the supplied commit message should be
+	cleaned up before committing. The '<mode>' can be `verbatim`,
+	`whitespace`, `strip`, or `default`.
++
+--
+default::
+	Strip leading and trailing empty lines and #commentary from
+	the commit message only if the message is to be edited.
+	Otherwise only remove whitespace.
+verbatim::
+	Do not change the message at all.
+whitespace::
+	Remove only leading and trailing whitespace lines.
+strip::
+	Remove both whitespace and commentary.
+--
++
+The default can be changed using the 'commit.cleanup' configuration
+variable (see linkgit:git-config[1]).
 
 -e::
 --edit::

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

* Re: [PATCH 4/4] Documentation/git-commit.txt: correct a few minor grammatical mistakes
  2013-02-19  6:43   ` Jonathan Nieder
@ 2013-02-19  7:18     ` Brandon Casey
  2013-02-19  7:29       ` Jonathan Nieder
  0 siblings, 1 reply; 19+ messages in thread
From: Brandon Casey @ 2013-02-19  7:18 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Ralf Thielow

On Mon, Feb 18, 2013 at 10:43 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Brandon Casey wrote:
>
>> --- a/Documentation/git-commit.txt
>> +++ b/Documentation/git-commit.txt
>> @@ -174,10 +174,10 @@ OPTIONS
>>  --cleanup=<mode>::
>>       This option sets how the commit message is cleaned up.
>>       The  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
>> -     and 'default'. The 'default' mode will strip leading and
>> +     or 'default'. The 'default' mode will strip leading and
>>       trailing empty lines and #commentary from the commit message
>> -     only if the message is to be edited. Otherwise only whitespace
>> -     removed. The 'verbatim' mode does not change message at all,
>> +     only if the message is to be edited. Otherwise only whitespace is
>> +     removed. The 'verbatim' mode does not change the message at all,
>>       'whitespace' removes just leading/trailing whitespace lines
>>       and 'strip' removes both whitespace and commentary. The default
>>       can be changed by the 'commit.cleanup' configuration variable
>
> Yeah, the current text is a bit choppy.  How about this?

Hmm, I think the original text was more confusing than I realized.  I
think we should reorder the cleanup modes, placing "default" last, and
then describe default in terms of either strip or whitespace depending
on whether an editor will be spawned.

> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>
> --- i/Documentation/git-commit.txt
> +++ w/Documentation/git-commit.txt
> @@ -172,16 +172,25 @@ OPTIONS
>         linkgit:git-commit-tree[1].
>
>  --cleanup=<mode>::
> -       This option sets how the commit message is cleaned up.
> -       The  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
> -       and 'default'. The 'default' mode will strip leading and
> -       trailing empty lines and #commentary from the commit message
> -       only if the message is to be edited. Otherwise only whitespace
> -       removed. The 'verbatim' mode does not change message at all,
> -       'whitespace' removes just leading/trailing whitespace lines
> -       and 'strip' removes both whitespace and commentary. The default
> -       can be changed by the 'commit.cleanup' configuration variable
> -       (see linkgit:git-config[1]).
> +       This option determines how the supplied commit message should be
> +       cleaned up before committing. The '<mode>' can be `verbatim`,
> +       `whitespace`, `strip`, or `default`.
> ++
> +--
> +default::
> +       Strip leading and trailing empty lines and #commentary from
> +       the commit message only if the message is to be edited.
> +       Otherwise only remove whitespace.
> +verbatim::
> +       Do not change the message at all.
> +whitespace::
> +       Remove only leading and trailing whitespace lines.
> +strip::
> +       Remove both whitespace and commentary.

Let's reorder these.  Maybe something like this:

+strip::
+       Strip leading and trailing empty lines, trailing whitespace
and #commentary and
+       collapse consecutive blank lines into one.
+whitespace::
+       Same as "strip" except #commentary is not removed.
+verbatim::
+       Do not change the message at all.
+default::
+       "strip" if the message is to be edited.  Otherwise "whitespace".

> +--
> ++
> +The default can be changed using the 'commit.cleanup' configuration
> +variable (see linkgit:git-config[1]).
>
>  -e::
>  --edit::

-Brandon

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

* Re: [PATCH 4/4] Documentation/git-commit.txt: correct a few minor grammatical mistakes
  2013-02-19  7:18     ` Brandon Casey
@ 2013-02-19  7:29       ` Jonathan Nieder
  2013-02-19 17:33         ` Junio C Hamano
  0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Nieder @ 2013-02-19  7:29 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git, Ralf Thielow

Brandon Casey wrote:

> Hmm, I think the original text was more confusing than I realized.  I
> think we should reorder the cleanup modes, placing "default" last, and
> then describe default in terms of either strip or whitespace depending
> on whether an editor will be spawned.

Sounds good to me. :)

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

* Re: [PATCH 1/4] t/t7502: compare entire commit message with what was expected
  2013-02-19  5:08 ` [PATCH 1/4] t/t7502: compare entire commit message with what was expected Jonathan Nieder
  2013-02-19  5:10   ` Jonathan Nieder
@ 2013-02-19 17:24   ` Junio C Hamano
  1 sibling, 0 replies; 19+ messages in thread
From: Junio C Hamano @ 2013-02-19 17:24 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Brandon Casey, git

Jonathan Nieder <jrnieder@gmail.com> writes:

> The downside (not a new problem, but a downside nonetheless) is that
> it means the test doesn't demonstrate what --cleanup=verbatim --status
> will do.
>
> How about something like this?

Can't we be a bit more robust by not using a hardcoded block of
lines as the "expect" string?  You could for example use what you
would see in your editor when "git commit" is run without the "-t"
option to form the expected pattern, no?

In any case, I think (1) a test for 'verbatim with status' is worth
doing, and (2) it would be cleaner to do this as a separate step,
perhaps on top of Brandon's 4-patch series.

>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>
> diff --git i/t/t7502-commit.sh w/t/t7502-commit.sh
> index cbd7a459..64162fce 100755
> --- i/t/t7502-commit.sh
> +++ w/t/t7502-commit.sh
> @@ -180,15 +180,37 @@ test_expect_success 'verbose respects diff config' '
>  test_expect_success 'cleanup commit messages (verbatim option,-t)' '
>  
>  	echo >>negative &&
> -	{ echo;echo "# text";echo; } >expect &&
> -	git commit --cleanup=verbatim -t expect -a &&
> -	git cat-file -p HEAD |sed -e "1,/^\$/d" |head -n 3 >actual &&
> +	{
> +		echo &&
> +		echo "# text" &&
> +		echo
> +	} >template &&
> +	{
> +		cat template &&
> +		cat <<-\EOF &&
> +
> +		# Please enter the commit message for your changes. Lines starting
> +		# with '\''#'\'' will be kept; you may remove them yourself if you want to.
> +		# An empty message aborts the commit.
> +		#
> +		# Author:    A U Thor <author@example.com>
> +		#
> +		EOF
> +		git commit -a --dry-run
> +	} >expect &&
> +	git commit --cleanup=verbatim -t template -a &&
> +	git cat-file -p HEAD |sed -e "1,/^\$/d" >actual &&
>  	test_cmp expect actual
>  
>  '
>  
>  test_expect_success 'cleanup commit messages (verbatim option,-F)' '
>  
> +	{
> +		echo &&
> +		echo "# text" &&
> +		echo
> +	} >expect &&
>  	echo >>negative &&
>  	git commit --cleanup=verbatim -F expect -a &&
>  	git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&

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

* Re: [PATCH 4/4] Documentation/git-commit.txt: correct a few minor grammatical mistakes
  2013-02-19  7:29       ` Jonathan Nieder
@ 2013-02-19 17:33         ` Junio C Hamano
  2013-02-19 18:14           ` [PATCH v2 4/4] Documentation/git-commit.txt: rework the --cleanup section Brandon Casey
                             ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Junio C Hamano @ 2013-02-19 17:33 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Brandon Casey, git, Ralf Thielow

Jonathan Nieder <jrnieder@gmail.com> writes:

> Brandon Casey wrote:
>
>> Hmm, I think the original text was more confusing than I realized.  I
>> think we should reorder the cleanup modes, placing "default" last, and
>> then describe default in terms of either strip or whitespace depending
>> on whether an editor will be spawned.
>
> Sounds good to me. :)

Will take 1-3 of the series for now, as the above seems to indicate
that I'll see a quick reroll of 4/4.

Thanks both for patches and review.

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

* [PATCH v2 4/4] Documentation/git-commit.txt: rework the --cleanup section
  2013-02-19 17:33         ` Junio C Hamano
@ 2013-02-19 18:14           ` Brandon Casey
  2013-02-19 18:28           ` Brandon Casey
  2013-02-19 18:29           ` Brandon Casey
  2 siblings, 0 replies; 19+ messages in thread
From: Brandon Casey @ 2013-02-19 18:14 UTC (permalink / raw)
  To: gitster; +Cc: git, jrnieder, ralf.thielow, Brandon Casey

From: Brandon Casey <drafnel@gmail.com>

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---

Ok, here's the updated text.  I am not set up to build the documentation,
so I hope someone will test, but looks right to me.

-Brandon


 Documentation/git-commit.txt | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0eb79cc..992c219 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -172,16 +172,24 @@ OPTIONS
        linkgit:git-commit-tree[1].
 
 --cleanup=<mode>::
-	This option sets how the commit message is cleaned up.
-	The  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
-	and 'default'. The 'default' mode will strip leading and
-	trailing empty lines and #commentary from the commit message
-	only if the message is to be edited. Otherwise only whitespace
-	removed. The 'verbatim' mode does not change message at all,
-	'whitespace' removes just leading/trailing whitespace lines
-	and 'strip' removes both whitespace and commentary. The default
-	can be changed by the 'commit.cleanup' configuration variable
-	(see linkgit:git-config[1]).
+	This option determines how the supplied commit message should be
+	cleaned up before committing.  The '<mode>' can be `strip`,
+	`whitespace`, `verbatim`, or `default`.
++
+--
+strip::
+	Strip leading and trailing empty lines, trailing whitespace, and
+	#commentary and collapse consecutive empty lines.
+whitespace::
+	Same as `strip` except #commentary is not removed.
+verbatim::
+	Do not change the message at all.
+default::
+	`strip` if the message is to be edited.  Otherwise `whitespace`.
+--
++
+	The default can be changed by the 'commit.cleanup' configuration
+	variable (see linkgit:git-config[1]).
 
 -e::
 --edit::
-- 
1.8.1.3.566.gaa39828

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

* [PATCH v2 4/4] Documentation/git-commit.txt: rework the --cleanup section
  2013-02-19 17:33         ` Junio C Hamano
  2013-02-19 18:14           ` [PATCH v2 4/4] Documentation/git-commit.txt: rework the --cleanup section Brandon Casey
@ 2013-02-19 18:28           ` Brandon Casey
  2013-02-19 18:29           ` Brandon Casey
  2 siblings, 0 replies; 19+ messages in thread
From: Brandon Casey @ 2013-02-19 18:28 UTC (permalink / raw)
  To: gitster; +Cc: git, jrnieder, ralf.thielow, Brandon Casey

From: Brandon Casey <drafnel@gmail.com>

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---


[RESEND] I originally specified Junio's address as gitster@pobox.org.

Ok, here's the updated text.  I am not set up to build the documentation,
so I hope someone will test, but looks right to me.

-Brandon


 Documentation/git-commit.txt | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0eb79cc..992c219 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -172,16 +172,24 @@ OPTIONS
        linkgit:git-commit-tree[1].
 
 --cleanup=<mode>::
-	This option sets how the commit message is cleaned up.
-	The  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
-	and 'default'. The 'default' mode will strip leading and
-	trailing empty lines and #commentary from the commit message
-	only if the message is to be edited. Otherwise only whitespace
-	removed. The 'verbatim' mode does not change message at all,
-	'whitespace' removes just leading/trailing whitespace lines
-	and 'strip' removes both whitespace and commentary. The default
-	can be changed by the 'commit.cleanup' configuration variable
-	(see linkgit:git-config[1]).
+	This option determines how the supplied commit message should be
+	cleaned up before committing.  The '<mode>' can be `strip`,
+	`whitespace`, `verbatim`, or `default`.
++
+--
+strip::
+	Strip leading and trailing empty lines, trailing whitespace, and
+	#commentary and collapse consecutive empty lines.
+whitespace::
+	Same as `strip` except #commentary is not removed.
+verbatim::
+	Do not change the message at all.
+default::
+	`strip` if the message is to be edited.  Otherwise `whitespace`.
+--
++
+	The default can be changed by the 'commit.cleanup' configuration
+	variable (see linkgit:git-config[1]).
 
 -e::
 --edit::
-- 
1.8.1.3.566.gaa39828

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

* [PATCH v2 4/4] Documentation/git-commit.txt: rework the --cleanup section
  2013-02-19 17:33         ` Junio C Hamano
  2013-02-19 18:14           ` [PATCH v2 4/4] Documentation/git-commit.txt: rework the --cleanup section Brandon Casey
  2013-02-19 18:28           ` Brandon Casey
@ 2013-02-19 18:29           ` Brandon Casey
  2013-02-19 20:28             ` [PATCH] fixup! " Jonathan Nieder
  2013-02-19 20:35             ` [PATCH v2 4/4] " Junio C Hamano
  2 siblings, 2 replies; 19+ messages in thread
From: Brandon Casey @ 2013-02-19 18:29 UTC (permalink / raw)
  To: gitster; +Cc: git, jrnieder, ralf.thielow, Brandon Casey

From: Brandon Casey <drafnel@gmail.com>

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---


[RESEND] I originally specified Junio's address as gitster@pobox.org.
[RESEND] Sorry, now with the correct address.

Ok, here's the updated text.  I am not set up to build the documentation,
so I hope someone will test, but looks right to me.

-Brandon


 Documentation/git-commit.txt | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0eb79cc..992c219 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -172,16 +172,24 @@ OPTIONS
        linkgit:git-commit-tree[1].
 
 --cleanup=<mode>::
-	This option sets how the commit message is cleaned up.
-	The  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
-	and 'default'. The 'default' mode will strip leading and
-	trailing empty lines and #commentary from the commit message
-	only if the message is to be edited. Otherwise only whitespace
-	removed. The 'verbatim' mode does not change message at all,
-	'whitespace' removes just leading/trailing whitespace lines
-	and 'strip' removes both whitespace and commentary. The default
-	can be changed by the 'commit.cleanup' configuration variable
-	(see linkgit:git-config[1]).
+	This option determines how the supplied commit message should be
+	cleaned up before committing.  The '<mode>' can be `strip`,
+	`whitespace`, `verbatim`, or `default`.
++
+--
+strip::
+	Strip leading and trailing empty lines, trailing whitespace, and
+	#commentary and collapse consecutive empty lines.
+whitespace::
+	Same as `strip` except #commentary is not removed.
+verbatim::
+	Do not change the message at all.
+default::
+	`strip` if the message is to be edited.  Otherwise `whitespace`.
+--
++
+	The default can be changed by the 'commit.cleanup' configuration
+	variable (see linkgit:git-config[1]).
 
 -e::
 --edit::
-- 
1.8.1.3.566.gaa39828

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

* [PATCH] fixup! Documentation/git-commit.txt: rework the --cleanup section
  2013-02-19 18:29           ` Brandon Casey
@ 2013-02-19 20:28             ` Jonathan Nieder
  2013-02-19 20:33               ` Brandon Casey
  2013-02-19 20:35             ` [PATCH v2 4/4] " Junio C Hamano
  1 sibling, 1 reply; 19+ messages in thread
From: Jonathan Nieder @ 2013-02-19 20:28 UTC (permalink / raw)
  To: Brandon Casey; +Cc: gitster, git, ralf.thielow, Brandon Casey

Hi,

Brandon Casey wrote:

> Signed-off-by: Brandon Casey <drafnel@gmail.com>

This renders as

	--cleanup=<mode>
	    This option determines how the supplied commit message
	    should be cleaned up before committing. The <mode> can be
	    strip, whitespace, verbatim, or default.

	    strip
		Strip leading and trailing empty lines, trailing
		whitespace, and #commentary and collapse consecutive
		empty lines.

	    whitespace
		Same as strip except #commentary is not removed.

	    verbatim
		Do not change the message at all.

	    default

		strip if the message is to be edited. Otherwise
		whitespace.

		The default can be changed by the 'commit.cleanup' config
		variable (see linkgit:git-config[1]).

Problems:

 * There's a weird extra blank line after "default"
 * Wrong indentation for the final paragraph.
 * The linkgit isn't resolved for some reason.

The following fixes it for me.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/git-commit.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 992c219..24a99cc 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -185,11 +185,12 @@ whitespace::
 verbatim::
 	Do not change the message at all.
 default::
-	`strip` if the message is to be edited.  Otherwise `whitespace`.
+	Same as `strip` if the message is to be edited.
+	Otherwise `whitespace`.
 --
 +
-	The default can be changed by the 'commit.cleanup' configuration
-	variable (see linkgit:git-config[1]).
+The default can be changed by the 'commit.cleanup' configuration
+variable (see linkgit:git-config[1]).
 
 -e::
 --edit::
-- 
1.8.1.3

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

* Re: [PATCH] fixup! Documentation/git-commit.txt: rework the --cleanup section
  2013-02-19 20:28             ` [PATCH] fixup! " Jonathan Nieder
@ 2013-02-19 20:33               ` Brandon Casey
  0 siblings, 0 replies; 19+ messages in thread
From: Brandon Casey @ 2013-02-19 20:33 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Brandon Casey, gitster, git, ralf.thielow

On Tue, Feb 19, 2013 at 12:28 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> Brandon Casey wrote:

> Problems:
>
>  * There's a weird extra blank line after "default"
>  * Wrong indentation for the final paragraph.
>  * The linkgit isn't resolved for some reason.
>
> The following fixes it for me.

Thanks Jonathan.

-Brandon

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

* Re: [PATCH v2 4/4] Documentation/git-commit.txt: rework the --cleanup section
  2013-02-19 18:29           ` Brandon Casey
  2013-02-19 20:28             ` [PATCH] fixup! " Jonathan Nieder
@ 2013-02-19 20:35             ` Junio C Hamano
  1 sibling, 0 replies; 19+ messages in thread
From: Junio C Hamano @ 2013-02-19 20:35 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git, jrnieder, ralf.thielow, Brandon Casey

Brandon Casey <bcasey@nvidia.com> writes:

> From: Brandon Casey <drafnel@gmail.com>
>
> Signed-off-by: Brandon Casey <drafnel@gmail.com>
> ---
>
>
> [RESEND] I originally specified Junio's address as gitster@pobox.org.
> [RESEND] Sorry, now with the correct address.
>
> Ok, here's the updated text.  I am not set up to build the documentation,
> so I hope someone will test, but looks right to me.

Thanks for marking this as unverified.

Jonathan, thanks for the fixup.

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

end of thread, other threads:[~2013-02-19 20:36 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-19  4:17 [PATCH 1/4] t/t7502: compare entire commit message with what was expected Brandon Casey
2013-02-19  4:17 ` [PATCH 2/4] t7502: demonstrate breakage with a commit message with trailing newlines Brandon Casey
2013-02-19  5:39   ` Jonathan Nieder
2013-02-19  4:17 ` [PATCH 3/4] git-commit: only append a newline to -m mesg if necessary Brandon Casey
2013-02-19  6:31   ` Jonathan Nieder
2013-02-19  4:17 ` [PATCH 4/4] Documentation/git-commit.txt: correct a few minor grammatical mistakes Brandon Casey
2013-02-19  6:43   ` Jonathan Nieder
2013-02-19  7:18     ` Brandon Casey
2013-02-19  7:29       ` Jonathan Nieder
2013-02-19 17:33         ` Junio C Hamano
2013-02-19 18:14           ` [PATCH v2 4/4] Documentation/git-commit.txt: rework the --cleanup section Brandon Casey
2013-02-19 18:28           ` Brandon Casey
2013-02-19 18:29           ` Brandon Casey
2013-02-19 20:28             ` [PATCH] fixup! " Jonathan Nieder
2013-02-19 20:33               ` Brandon Casey
2013-02-19 20:35             ` [PATCH v2 4/4] " Junio C Hamano
2013-02-19  5:08 ` [PATCH 1/4] t/t7502: compare entire commit message with what was expected Jonathan Nieder
2013-02-19  5:10   ` Jonathan Nieder
2013-02-19 17:24   ` Junio C Hamano

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.