All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Clarify documentation on commit message strip
@ 2015-04-24 20:06 Fredrik Gustafsson
  2015-04-25  4:36 ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Fredrik Gustafsson @ 2015-04-24 20:06 UTC (permalink / raw)
  To: iveqy; +Cc: git

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

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

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

* Re: [PATCH] Clarify documentation on commit message strip
  2015-04-24 20:06 [PATCH] Clarify documentation on commit message strip Fredrik Gustafsson
@ 2015-04-25  4:36 ` Junio C Hamano
  2015-04-25  5:42   ` Fredrik Gustafsson
  0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2015-04-25  4:36 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: git

Fredrik Gustafsson <iveqy@iveqy.com> writes:

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

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

* [PATCH] Clarify documentation on commit message strip
  2015-04-25  4:36 ` Junio C Hamano
@ 2015-04-25  5:42   ` Fredrik Gustafsson
  2015-04-27 17:31     ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Fredrik Gustafsson @ 2015-04-25  5:42 UTC (permalink / raw)
  To: git; +Cc: gitster, iveqy

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

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

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

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

* Re: [PATCH] Clarify documentation on commit message strip
  2015-04-25  5:42   ` Fredrik Gustafsson
@ 2015-04-27 17:31     ` Junio C Hamano
  2015-04-27 18:13       ` Fredrik Gustafsson
  0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2015-04-27 17:31 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: git

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

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

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

* Re: [PATCH] Clarify documentation on commit message strip
  2015-04-27 17:31     ` Junio C Hamano
@ 2015-04-27 18:13       ` Fredrik Gustafsson
  0 siblings, 0 replies; 5+ messages in thread
From: Fredrik Gustafsson @ 2015-04-27 18:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Fredrik Gustafsson, git

On Mon, Apr 27, 2015 at 10:31:28AM -0700, Junio C Hamano wrote:
> 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.

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

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

end of thread, other threads:[~2015-04-27 18:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-24 20:06 [PATCH] Clarify documentation on commit message strip Fredrik Gustafsson
2015-04-25  4:36 ` Junio C Hamano
2015-04-25  5:42   ` Fredrik Gustafsson
2015-04-27 17:31     ` Junio C Hamano
2015-04-27 18:13       ` Fredrik Gustafsson

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.