threads / patch / 39156

patchClarify documentation on commit message strip

Subject: [PATCH] Clarify documentation on commit message strip

## tl;dr

5 messages between Apr 24, 2015 and Apr 27, 2015. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Fredrik Gustafsson· Apr 24, 2015, 20:06 UTC · lore

When using `-v` to add a unified diff to the commit message, this is stripped and not a part of the commit message. This is not mentioned.

Add a note about this with the `-v` description as well as description for the default `--cleanup` mode.

Signed-off-by: Fredrik Gustafsson <iveqy@iveqy.com>
---
 Documentation/git-commit.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Show changes to Documentation/git-commit.txt +5 −3

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 617e29b..e31d828 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -180,8 +180,9 @@ OPTIONS
 +
 --
 strip::
-	Strip leading and trailing empty lines, trailing whitespace, and
-	#commentary and collapse consecutive empty lines.
+	Strip leading and trailing empty lines, trailing whitespace,
+	#commentary, unified diff added with `-v` and collapse
+	consecutive empty lines.
 whitespace::
 	Same as `strip` except #commentary is not removed.
 verbatim::
@@ -283,7 +284,8 @@ configuration variable documented in linkgit:git-config[1].
 	Show unified diff between the HEAD commit and what
 	would be committed at the bottom of the commit message
 	template.  Note that this diff output doesn't have its
-	lines prefixed with '#'.
+	lines prefixed with '#'. This diff will not be a part
+	of the commit message.
 +
 If specified twice, show in addition the unified diff between
 what would be committed and the worktree files, i.e. the unstaged

-- 
2.1.4

Junio C Hamano· Apr 25, 2015, 04:36 UTC · re: Fredrik Gustafsson · lore

Re: [PATCH] Clarify documentation on commit message strip

Fredrik Gustafsson <iveqy@iveqy.com> writes:

Show 13 quoted lines

> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index 617e29b..e31d828 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -180,8 +180,9 @@ OPTIONS
>  +
>  --
>  strip::
> -	Strip leading and trailing empty lines, trailing whitespace, and
> -	#commentary and collapse consecutive empty lines.
> +	Strip leading and trailing empty lines, trailing whitespace,
> +	#commentary, unified diff added with `-v` and collapse
> +	consecutive empty lines.

I'd prefer the description not to be _too_ explicit e.g. by mentioning "unified diff", etc.

Personally I think it is sufficient to do s/#comment/comment/ to the existing text, without doing anything else. What is "commentary" to be removed is fairly clear in the contents given to the user in the editor.

Fredrik Gustafsson· Apr 25, 2015, 05:42 UTC · re: Junio C Hamano · lore

When using `-v` to add a unified diff to the commit message, this is stripped and not a part of the commit message. This is not mentioned.

Add a note about this with the `-v` description and slightly modify the description for the default `--cleanup` mode.

Signed-off-by: Fredrik Gustafsson <iveqy@iveqy.com>
---

Show 7 quoted lines

>I'd prefer the description not to be _too_ explicit e.g. by
>mentioning "unified diff", etc.
>
>Personally I think it is sufficient to do s/#comment/comment/ to the
>existing text, without doing anything else.  What is "commentary" to
>be removed is fairly clear in the contents given to the user in the
>editor.

I agree that it is very clear once you do edit the commit message. My main point with this patch was to clarify -v, since it's not obvious from the documentation that it will be removed.

I've no objections about your suggestions about the `strip` part. I hope I've understood you correctly and that this patch is correct.

 Documentation/git-commit.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Show changes to Documentation/git-commit.txt +4 −3

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 617e29b..1db4c7f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -180,8 +180,8 @@ OPTIONS
 +
 --
 strip::
-	Strip leading and trailing empty lines, trailing whitespace, and
-	#commentary and collapse consecutive empty lines.
+	Strip leading and trailing empty lines, trailing whitespace,
+	commentary and collapse consecutive empty lines.
 whitespace::
 	Same as `strip` except #commentary is not removed.
 verbatim::
@@ -283,7 +283,8 @@ configuration variable documented in linkgit:git-config[1].
 	Show unified diff between the HEAD commit and what
 	would be committed at the bottom of the commit message
 	template.  Note that this diff output doesn't have its
-	lines prefixed with '#'.
+	lines prefixed with '#'. This diff will not be a part
+	of the commit message.
 +
 If specified twice, show in addition the unified diff between
 what would be committed and the worktree files, i.e. the unstaged

-- 
2.1.4

Junio C Hamano· Apr 27, 2015, 17:31 UTC · re: Fredrik Gustafsson · lore

Re: [PATCH] Clarify documentation on commit message strip

Fredrik Gustafsson <iveqy@iveqy.com> writes:

> I agree that it is very clear once you do edit the commit message. My main
> point with this patch was to clarify -v, since it's not obvious from the
> documentation that it will be removed.

While I agree with you that the documentation should make it clear that the patch will not be part of the commit, I think the root cause of the current "unclear-ness" is because the documentation does not make it clear *WHY* the "-v" option gives the patch text there.

Doing something like this on top of your patch may clarify the reason what "-v" is used for, and I suspect that it may even make it unnecessary to explicitly say that the patch text will not be part of the log message (but I didn't remove that with this "how about this" follow-up).

 Documentation/git-commit.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Show changes to Documentation/git-commit.txt +3 −1

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 1db4c7f..617dea0 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -282,7 +282,9 @@ configuration variable documented in linkgit:git-config[1].
 --verbose::
 	Show unified diff between the HEAD commit and what
 	would be committed at the bottom of the commit message
-	template.  Note that this diff output doesn't have its
+	template to help the user describe the commit by reminding
+	what changes the commit has.
+	Note that this diff output doesn't have its
 	lines prefixed with '#'. This diff will not be a part
 	of the commit message.
 +

Fredrik Gustafsson· Apr 27, 2015, 18:13 UTC · re: Junio C Hamano · lore

Re: [PATCH] Clarify documentation on commit message strip

On Mon, Apr 27, 2015 at 10:31:28AM -0700, Junio C Hamano wrote:

Show 11 quoted lines

> Fredrik Gustafsson <iveqy@iveqy.com> writes:
> 
> > I agree that it is very clear once you do edit the commit message. My main
> > point with this patch was to clarify -v, since it's not obvious from the
> > documentation that it will be removed.
> 
> While I agree with you that the documentation should make it clear
> that the patch will not be part of the commit, I think the root
> cause of the current "unclear-ness" is because the documentation
> does not make it clear *WHY* the "-v" option gives the patch text
> there.

I agree.

Show 6 quoted lines

> 
> Doing something like this on top of your patch may clarify the
> reason what "-v" is used for, and I suspect that it may even make it
> unnecessary to explicitly say that the patch text will not be part
> of the log message (but I didn't remove that with this "how about
> this" follow-up).

Even better. I would be glad if this could make it into a release. Thanks for the feedback, to make this a good patch.

-- 
Fredrik Gustafsson

phone: +46 733-608274
e-mail: iveqy@iveqy.com
website: http://www.iveqy.com