git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] format-patch: Add a signature option (--signature)
@ 2010-06-15  5:00 Stephen Boyd
  2010-06-15  5:00 ` [PATCH 2/2] completion: Add --signoff and format.signoff Stephen Boyd
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Stephen Boyd @ 2010-06-15  5:00 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

By default, git uses the version string as the signature for all
patches output by format-patch. Many employers (mine included)
require the use of a signature on all outgoing mails. In a
format-patch | send-email workflow there isn't an easy way to modify
the signature without breaking the pipe and manually replacing the
version string with the signature required. Instead of doing all that
work, add an option (--signature) and a config variable
(format.signature) to replace the default git version signature when
formatting patches.

This does modify the original behavior of format-patch a bit. First
off the version string is now placed in the cover letter by default.
Secondly, once the configuration variable format.signature is added
to the .config file there is no way to revert back to the default
git version signature. Instead, specifying the --no-signature option
will remove the signature from the patches entirely.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---

I didn't cover the attachment case since there wasn't a signature there
before. Maybe later.

 Documentation/config.txt                           |    5 +++
 Documentation/git-format-patch.txt                 |    8 ++++-
 builtin/log.c                                      |   18 ++++++++--
 ...tch_--stdout_--cover-letter_-n_initial..master^ |    3 ++
 t/t4014-format-patch.sh                            |   35 ++++++++++++++++++++
 5 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 95cf73c..28379cc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -880,6 +880,11 @@ format.subjectprefix::
 	The default for format-patch is to output files with the '[PATCH]'
 	subject prefix. Use this variable to change that prefix.
 
+format.signature::
+	The default for format-patch is to output a signature containing
+	the git version number. Use this variable to change the default
+	signature.
+
 format.suffix::
 	The default for format-patch is to output files with the suffix
 	`.patch`. Use this variable to change that suffix (make sure to
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 835fb71..9a1b1d0 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git format-patch' [-k] [(-o|--output-directory) <dir> | --stdout]
 		   [--no-thread | --thread[=<style>]]
 		   [(--attach|--inline)[=<boundary>] | --no-attach]
-		   [-s | --signoff]
+		   [-s | --signoff] [--signature=<signature>]
 		   [-n | --numbered | -N | --no-numbered]
 		   [--start-number <n>] [--numbered-files]
 		   [--in-reply-to=Message-Id] [--suffix=.<sfx>]
@@ -180,6 +180,12 @@ will want to ensure that threading is disabled for `git send-email`.
 	containing the shortlog and the overall diffstat.  You can
 	fill in a description in the file before sending it out.
 
+--signature=<signature>::
+	Add a signature to each patch and, if --cover-letter is specified,
+	the cover letter. Per RFC 3676 the signature is separated from the
+	body by '-- '. If no signature option is given, the signature defaults
+	to the git version number.
+
 --suffix=.<sfx>::
 	Instead of using `.patch` as the suffix for generated
 	filenames, use specified suffix.  A common alternative is
diff --git a/builtin/log.c b/builtin/log.c
index 976e16f..b78027e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -549,8 +549,9 @@ static void add_header(const char *value)
 
 #define THREAD_SHALLOW 1
 #define THREAD_DEEP 2
-static int thread = 0;
-static int do_signoff = 0;
+static int thread;
+static int do_signoff;
+static const char *signature = git_version_string;
 
 static int git_format_config(const char *var, const char *value, void *cb)
 {
@@ -609,6 +610,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
 		do_signoff = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "format.signature"))
+		return git_config_string(&signature, var, value);
 
 	return git_log_config(var, value, cb);
 }
@@ -703,6 +706,12 @@ static void gen_message_id(struct rev_info *info, char *base)
 	info->message_id = strbuf_detach(&buf, NULL);
 }
 
+static void print_signature(void)
+{
+	if (signature)
+		printf("-- \n%s\n\n", signature);
+}
+
 static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			      int numbered, int numbered_files,
 			      struct commit *origin,
@@ -796,6 +805,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	diff_flush(&opts);
 
 	printf("\n");
+	print_signature();
 }
 
 static const char *clean_message_id(const char *msg_id)
@@ -1035,6 +1045,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		{ OPTION_CALLBACK, 0, "thread", &thread, "style",
 			    "enable message threading, styles: shallow, deep",
 			    PARSE_OPT_OPTARG, thread_callback },
+		OPT_STRING(0, "signature", &signature, "signature",
+			    "add a signature"),
 		OPT_END()
 	};
 
@@ -1313,7 +1325,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 				       mime_boundary_leader,
 				       rev.mime_boundary);
 			else
-				printf("-- \n%s\n\n", git_version_string);
+				print_signature();
 		}
 		if (!use_stdout)
 			fclose(stdout);
diff --git a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
index 8dab4bf..1f0f9ad 100644
--- a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
+++ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
@@ -18,6 +18,9 @@ A U Thor (2):
  create mode 100644 file1
  delete mode 100644 file2
 
+-- 
+g-i-t--v-e-r-s-i-o-n
+
 From 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 Mon Sep 17 00:00:00 2001
 From: A U Thor <author@example.com>
 Date: Mon, 26 Jun 2006 00:01:00 +0000
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index d21c37f..06c481f 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -613,4 +613,39 @@ test_expect_success 'format-patch --ignore-if-in-upstream HEAD' '
 	git format-patch --ignore-if-in-upstream HEAD
 '
 
+test_expect_success 'format-patch --signature' '
+	git format-patch --stdout --signature="my sig" -1 >output &&
+	grep "my sig" output
+'
+
+test_expect_success 'format-patch with format.signature config' '
+	git config format.signature "config sig" &&
+	git format-patch --stdout -1 >output &&
+	grep "config sig" output
+'
+
+test_expect_success 'format-patch --signature overrides format.signature' '
+	git config format.signature "config sig" &&
+	git format-patch --stdout --signature="overrides" -1 >output &&
+	! grep "config sig" output &&
+	grep "overrides" output
+'
+
+test_expect_success 'format-patch --no-signature ignores format.signature' '
+	git config format.signature "config sig" &&
+	git format-patch --stdout --signature="my sig" --no-signature \
+		-1 >output &&
+	! grep "config sig" output &&
+	! grep "my sig" output &&
+	! grep "^-- \$" output
+'
+
+test_expect_success 'format-patch --signature --cover-letter' '
+	git config --unset-all format.signature &&
+	git format-patch --stdout --signature="my sig" --cover-letter \
+		-1 >output &&
+	grep "my sig" output &&
+	test 2 = $(grep "my sig" output | wc -l)
+'
+
 test_done
-- 
1.7.1.257.g678728

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

* [PATCH 2/2] completion: Add --signoff and format.signoff
  2010-06-15  5:00 [PATCH 1/2] format-patch: Add a signature option (--signature) Stephen Boyd
@ 2010-06-15  5:00 ` Stephen Boyd
  2010-06-15 17:05 ` [PATCH 1/2] format-patch: Add a signature option (--signature) Junio C Hamano
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2010-06-15  5:00 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Shawn O. Pearce

Cc: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---

Feel free to squash.

 contrib/completion/git-completion.bash |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 57245a8..9ca54e0 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1052,7 +1052,7 @@ _git_format_patch ()
 			--numbered --start-number
 			--numbered-files
 			--keep-subject
-			--signoff
+			--signoff --signature
 			--in-reply-to= --cc=
 			--full-index --binary
 			--not --all
@@ -1726,6 +1726,7 @@ _git_config ()
 		format.headers
 		format.numbered
 		format.pretty
+		format.signature
 		format.signoff
 		format.subjectprefix
 		format.suffix
-- 
1.7.1.257.g678728

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

* Re: [PATCH 1/2] format-patch: Add a signature option (--signature)
  2010-06-15  5:00 [PATCH 1/2] format-patch: Add a signature option (--signature) Stephen Boyd
  2010-06-15  5:00 ` [PATCH 2/2] completion: Add --signoff and format.signoff Stephen Boyd
@ 2010-06-15 17:05 ` Junio C Hamano
  2010-06-16  4:31   ` Stephen Boyd
  2010-06-16  5:59 ` [PATCHv2 " Stephen Boyd
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2010-06-15 17:05 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: git

Stephen Boyd <bebarino@gmail.com> writes:

> This does modify the original behavior of format-patch a bit. First
> off the version string is now placed in the cover letter by default.

That part is fine; I think it was an oversight of the cover letter
addition.

> Secondly, once the configuration variable format.signature is added
> to the .config file there is no way to revert back to the default
> git version signature. Instead, specifying the --no-signature option
> will remove the signature from the patches entirely.

Hmph.  That is somewhat sad, but it probably is Ok.

> @@ -180,6 +180,12 @@ will want to ensure that threading is disabled for `git send-email`.
>  	containing the shortlog and the overall diffstat.  You can
>  	fill in a description in the file before sending it out.
>  
> +--signature=<signature>::
> +	Add a signature to each patch and, if --cover-letter is specified,
> +	the cover letter. Per RFC 3676 the signature is separated from the
> +	body by '-- '.

Wouldn't "Add a signature to each message produced" be easier to understand?
Also perhaps s/body by '-- '/& a line with '-- ' on it/?

Don't we want either an extra header to this entry ("--no-signature::")
and advertise it as a way to disable signature generation?

> diff --git a/builtin/log.c b/builtin/log.c
> index 976e16f..b78027e 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -609,6 +610,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
>  		do_signoff = git_config_bool(var, value);
>  		return 0;
>  	}
> +	if (!strcmp(var, "format.signature"))
> +		return git_config_string(&signature, var, value);
>  
>  	return git_log_config(var, value, cb);
>  }

This, together with ...

> @@ -703,6 +706,12 @@ static void gen_message_id(struct rev_info *info, char *base)
>  	info->message_id = strbuf_detach(&buf, NULL);
>  }
>  
> +static void print_signature(void)
> +{
> +	if (signature)
> +		printf("-- \n%s\n\n", signature);
> +}
> +

... this, and ...

> @@ -1035,6 +1045,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  		{ OPTION_CALLBACK, 0, "thread", &thread, "style",
>  			    "enable message threading, styles: shallow, deep",
>  			    PARSE_OPT_OPTARG, thread_callback },
> +		OPT_STRING(0, "signature", &signature, "signature",
> +			    "add a signature"),
>  		OPT_END()
>  	};

... this would mean that

 (1) these behave differently:

	format-patch --no-signature -1 HEAD
        format-patch --signature= -1 HEAD

 (2) this behaves like the second one in the above:

	[format]
        	signature = ""

 (3) there is no way to say "no signature, ever" in the configuration.

Perhaps we would want to do this instead?

        static void print_signature(void)
        {
                if (signature && *signature)
                        printf("-- \n%s\n\n", signature);
        }

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

* Re: [PATCH 1/2] format-patch: Add a signature option (--signature)
  2010-06-15 17:05 ` [PATCH 1/2] format-patch: Add a signature option (--signature) Junio C Hamano
@ 2010-06-16  4:31   ` Stephen Boyd
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2010-06-16  4:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On 06/15/2010 10:05 AM, Junio C Hamano wrote:
> Stephen Boyd <bebarino@gmail.com> write
>> @@ -180,6 +180,12 @@ will want to ensure that threading is disabled for `git send-email`.
>>  	containing the shortlog and the overall diffstat.  You can
>>  	fill in a description in the file before sending it out.
>>  
>> +--signature=<signature>::
>> +	Add a signature to each patch and, if --cover-letter is specified,
>> +	the cover letter. Per RFC 3676 the signature is separated from the
>> +	body by '-- '.
>
> Wouldn't "Add a signature to each message produced" be easier to understand?
> Also perhaps s/body by '-- '/& a line with '-- ' on it/?
>
> Don't we want either an extra header to this entry ("--no-signature::")
> and advertise it as a way to disable signature generation?

Both sound good.

>
>  (3) there is no way to say "no signature, ever" in the configuration.
>
> Perhaps we would want to do this instead?
>
>         static void print_signature(void)
>         {
>                 if (signature && *signature)
>                         printf("-- \n%s\n\n", signature);
>         }

Sounds good too. It's not entirely obvious that format.signature=""
means no signature ever, but that's where documentation steps in I guess.

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

* [PATCHv2 1/2] format-patch: Add a signature option (--signature)
  2010-06-15  5:00 [PATCH 1/2] format-patch: Add a signature option (--signature) Stephen Boyd
  2010-06-15  5:00 ` [PATCH 2/2] completion: Add --signoff and format.signoff Stephen Boyd
  2010-06-15 17:05 ` [PATCH 1/2] format-patch: Add a signature option (--signature) Junio C Hamano
@ 2010-06-16  5:59 ` Stephen Boyd
  2010-06-16  5:59 ` [PATCHv2 2/2] completion: Add --signature and format.signature Stephen Boyd
  2010-06-16 13:13 ` [PATCH 1/2] format-patch: Add a signature option (--signature) Theodore Tso
  4 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2010-06-16  5:59 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

By default, git uses the version string as the signature for all
patches output by format-patch. Many employers (mine included)
require the use of a signature on all outgoing mails. In a
format-patch | send-email workflow there isn't an easy way to modify
the signature without breaking the pipe and manually replacing the
version string with the signature required. Instead of doing all that
work, add an option (--signature) and a config variable
(format.signature) to replace the default git version signature when
formatting patches.

This does modify the original behavior of format-patch a bit. First
off the version string is now placed in the cover letter by default.
Secondly, once the configuration variable format.signature is added
to the .config file there is no way to revert back to the default
git version signature. Instead, specifying the --no-signature option
will remove the signature from the patches entirely.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---

 Changes since v1:
  - Logic to cover empty string case
  - Documentation updates
  - Tests for empty string behavior

 Documentation/config.txt                           |    6 ++
 Documentation/git-format-patch.txt                 |    7 +++
 builtin/log.c                                      |   18 ++++++-
 ...tch_--stdout_--cover-letter_-n_initial..master^ |    3 +
 t/t4014-format-patch.sh                            |   52 ++++++++++++++++++++
 5 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 95cf73c..be6e85d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -880,6 +880,12 @@ format.subjectprefix::
 	The default for format-patch is to output files with the '[PATCH]'
 	subject prefix. Use this variable to change that prefix.
 
+format.signature::
+	The default for format-patch is to output a signature containing
+	the git version number. Use this variable to change that default.
+	Set this variable to the empty string ("") to suppress
+	signature generation.
+
 format.suffix::
 	The default for format-patch is to output files with the suffix
 	`.patch`. Use this variable to change that suffix (make sure to
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 835fb71..c8c81e8 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -13,6 +13,7 @@ SYNOPSIS
 		   [--no-thread | --thread[=<style>]]
 		   [(--attach|--inline)[=<boundary>] | --no-attach]
 		   [-s | --signoff]
+		   [--signature=<signature> | --no-signature]
 		   [-n | --numbered | -N | --no-numbered]
 		   [--start-number <n>] [--numbered-files]
 		   [--in-reply-to=Message-Id] [--suffix=.<sfx>]
@@ -180,6 +181,12 @@ will want to ensure that threading is disabled for `git send-email`.
 	containing the shortlog and the overall diffstat.  You can
 	fill in a description in the file before sending it out.
 
+--[no]-signature=<signature>::
+	Add a signature to each message produced. Per RFC 3676 the signature
+	is separated from the body by a line with '-- ' on it. If the
+	signature option is omitted the signature defaults to the git version
+	number.
+
 --suffix=.<sfx>::
 	Instead of using `.patch` as the suffix for generated
 	filenames, use specified suffix.  A common alternative is
diff --git a/builtin/log.c b/builtin/log.c
index 976e16f..f068583 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -549,8 +549,9 @@ static void add_header(const char *value)
 
 #define THREAD_SHALLOW 1
 #define THREAD_DEEP 2
-static int thread = 0;
-static int do_signoff = 0;
+static int thread;
+static int do_signoff;
+static const char *signature = git_version_string;
 
 static int git_format_config(const char *var, const char *value, void *cb)
 {
@@ -609,6 +610,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
 		do_signoff = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "format.signature"))
+		return git_config_string(&signature, var, value);
 
 	return git_log_config(var, value, cb);
 }
@@ -703,6 +706,12 @@ static void gen_message_id(struct rev_info *info, char *base)
 	info->message_id = strbuf_detach(&buf, NULL);
 }
 
+static void print_signature(void)
+{
+	if (signature && *signature)
+		printf("-- \n%s\n\n", signature);
+}
+
 static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			      int numbered, int numbered_files,
 			      struct commit *origin,
@@ -796,6 +805,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	diff_flush(&opts);
 
 	printf("\n");
+	print_signature();
 }
 
 static const char *clean_message_id(const char *msg_id)
@@ -1035,6 +1045,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		{ OPTION_CALLBACK, 0, "thread", &thread, "style",
 			    "enable message threading, styles: shallow, deep",
 			    PARSE_OPT_OPTARG, thread_callback },
+		OPT_STRING(0, "signature", &signature, "signature",
+			    "add a signature"),
 		OPT_END()
 	};
 
@@ -1313,7 +1325,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 				       mime_boundary_leader,
 				       rev.mime_boundary);
 			else
-				printf("-- \n%s\n\n", git_version_string);
+				print_signature();
 		}
 		if (!use_stdout)
 			fclose(stdout);
diff --git a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
index 8dab4bf..1f0f9ad 100644
--- a/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
+++ b/t/t4013/diff.format-patch_--stdout_--cover-letter_-n_initial..master^
@@ -18,6 +18,9 @@ A U Thor (2):
  create mode 100644 file1
  delete mode 100644 file2
 
+-- 
+g-i-t--v-e-r-s-i-o-n
+
 From 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44 Mon Sep 17 00:00:00 2001
 From: A U Thor <author@example.com>
 Date: Mon, 26 Jun 2006 00:01:00 +0000
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index d21c37f..f87434b 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -613,4 +613,56 @@ test_expect_success 'format-patch --ignore-if-in-upstream HEAD' '
 	git format-patch --ignore-if-in-upstream HEAD
 '
 
+test_expect_success 'format-patch --signature' '
+	git format-patch --stdout --signature="my sig" -1 >output &&
+	grep "my sig" output
+'
+
+test_expect_success 'format-patch with format.signature config' '
+	git config format.signature "config sig" &&
+	git format-patch --stdout -1 >output &&
+	grep "config sig" output
+'
+
+test_expect_success 'format-patch --signature overrides format.signature' '
+	git config format.signature "config sig" &&
+	git format-patch --stdout --signature="overrides" -1 >output &&
+	! grep "config sig" output &&
+	grep "overrides" output
+'
+
+test_expect_success 'format-patch --no-signature ignores format.signature' '
+	git config format.signature "config sig" &&
+	git format-patch --stdout --signature="my sig" --no-signature \
+		-1 >output &&
+	! grep "config sig" output &&
+	! grep "my sig" output &&
+	! grep "^-- \$" output
+'
+
+test_expect_success 'format-patch --signature --cover-letter' '
+	git config --unset-all format.signature &&
+	git format-patch --stdout --signature="my sig" --cover-letter \
+		-1 >output &&
+	grep "my sig" output &&
+	test 2 = $(grep "my sig" output | wc -l)
+'
+
+test_expect_success 'format.signature="" supresses signatures' '
+	git config format.signature "" &&
+	git format-patch --stdout -1 >output &&
+	! grep "^-- \$" output
+'
+
+test_expect_success 'format-patch --no-signature supresses signatures' '
+	git config --unset-all format.signature &&
+	git format-patch --stdout --no-signature -1 >output &&
+	! grep "^-- \$" output
+'
+
+test_expect_success 'format-patch --signature="" supresses signatures' '
+	git format-patch --signature="" -1 >output &&
+	! grep "^-- \$" output
+'
+
 test_done
-- 
1.7.1.257.g678728

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

* [PATCHv2 2/2] completion: Add --signature and format.signature
  2010-06-15  5:00 [PATCH 1/2] format-patch: Add a signature option (--signature) Stephen Boyd
                   ` (2 preceding siblings ...)
  2010-06-16  5:59 ` [PATCHv2 " Stephen Boyd
@ 2010-06-16  5:59 ` Stephen Boyd
  2010-06-16 13:13 ` [PATCH 1/2] format-patch: Add a signature option (--signature) Theodore Tso
  4 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2010-06-16  5:59 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Shawn O. Pearce

Cc: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---

 Changes since v1:
  - Fixed subject (hah --signoff was totally wrong)

 contrib/completion/git-completion.bash |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 57245a8..d950776 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1052,7 +1052,7 @@ _git_format_patch ()
 			--numbered --start-number
 			--numbered-files
 			--keep-subject
-			--signoff
+			--signoff --signature --no-signature
 			--in-reply-to= --cc=
 			--full-index --binary
 			--not --all
@@ -1726,6 +1726,7 @@ _git_config ()
 		format.headers
 		format.numbered
 		format.pretty
+		format.signature
 		format.signoff
 		format.subjectprefix
 		format.suffix
-- 
1.7.1.257.g678728

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

* Re: [PATCH 1/2] format-patch: Add a signature option (--signature)
  2010-06-15  5:00 [PATCH 1/2] format-patch: Add a signature option (--signature) Stephen Boyd
                   ` (3 preceding siblings ...)
  2010-06-16  5:59 ` [PATCHv2 2/2] completion: Add --signature and format.signature Stephen Boyd
@ 2010-06-16 13:13 ` Theodore Tso
  2010-06-16 16:53   ` Junio C Hamano
  4 siblings, 1 reply; 10+ messages in thread
From: Theodore Tso @ 2010-06-16 13:13 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: git, Junio C Hamano


On Jun 15, 2010, at 1:00 AM, Stephen Boyd wrote:

> 
> This does modify the original behavior of format-patch a bit. First
> off the version string is now placed in the cover letter by default.

I don't know how important people will feel this to be, but I've occasionally
found it interesting to see how many people are using various different
versions of git in a particular development community, and having the
version in the signature is a useful gauge on that.  Putting it in the cover
letter isn't really a complete substitute for this because many patches
and short patch series go out without cover letters...

-- Ted

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

* Re: [PATCH 1/2] format-patch: Add a signature option (--signature)
  2010-06-16 13:13 ` [PATCH 1/2] format-patch: Add a signature option (--signature) Theodore Tso
@ 2010-06-16 16:53   ` Junio C Hamano
  2010-06-16 19:43     ` Stephen Boyd
  0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2010-06-16 16:53 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Stephen Boyd, git

Theodore Tso <tytso@MIT.EDU> writes:

> On Jun 15, 2010, at 1:00 AM, Stephen Boyd wrote:
>> 
>> This does modify the original behavior of format-patch a bit. First
>> off the version string is now placed in the cover letter by default.
>
> I don't know how important people will feel this to be, but I've occasionally
> found it interesting to see how many people are using various different
> versions of git in a particular development community, and having the
> version in the signature is a useful gauge on that.

The original motivation of the version signature was exactly that ;-).

> Putting it in the cover
> letter isn't really a complete substitute for this because many patches
> and short patch series go out without cover letters...

I think you misread Stephen, he misspoke, or I misread the patch.  The
intention of the change as I understand it is to put the signature in the
cover letter, in addition to the patches.

One bad thing about having the "signature" line at the end of patch is
that various third party tools mistake it as one extra deleted line.  One
offender that is most often seen to corrupt patches is Emacs diff mode.
Doing an equivalent of "recountdiff" after it allows the user to edit a
patch is fine, but it doesn't bother to remember what was in the original
and what was added/deleted by the user, and instead seems to just recount
the result from scratch.

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

* Re: [PATCH 1/2] format-patch: Add a signature option (--signature)
  2010-06-16 16:53   ` Junio C Hamano
@ 2010-06-16 19:43     ` Stephen Boyd
  2010-06-16 20:55       ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Boyd @ 2010-06-16 19:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Theodore Tso, git

On Wed, Jun 16, 2010 at 9:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Theodore Tso <tytso@MIT.EDU> writes:
>
>>
>> I don't know how important people will feel this to be, but I've occasionally
>> found it interesting to see how many people are using various different
>> versions of git in a particular development community, and having the
>> version in the signature is a useful gauge on that.
>
> The original motivation of the version signature was exactly that ;-).
>

At least with git send-email you get the X-Mailer header and you know
what version is being used. I admit that isn't as visible/useful as
the signature though and not everyone is using git send-email.

>
> I think you misread Stephen, he misspoke, or I misread the patch.  The
> intention of the change as I understand it is to put the signature in the
> cover letter, in addition to the patches.
>

Yes, this is one intention. The other intention is to allow me to
customize what that signature is (or to have no signature at all).
Currently it replaces the version number.

The other option is appending the signature to the version number, but
I thought it looked funny:

    --
    1.7.0

    Signature goes here

but now that I look at it again maybe it looks ok.

If the answer is there should always be the version number in the
signature then renaming --signature to --custom-signature or
--extra-signature or --append-signature would be more clear (albeit
verbose). Then  --no-*-signature wouldn't imply that you are removing
the default version signature (because you can't).

If the signature is causing problems for third-party tools then maybe
in some projects they wouldn't want the signature at all and
--no-signature would be a good option there. It doesn't mean we have
to add a --signature=<str> option too, instead it could just be a
boolean option.

The real question is how useful is it to have the version number
there? There are other ways to gauge the version usage and it's opt-in
so you'll still get some version number coverage.

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

* Re: [PATCH 1/2] format-patch: Add a signature option (--signature)
  2010-06-16 19:43     ` Stephen Boyd
@ 2010-06-16 20:55       ` Junio C Hamano
  0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2010-06-16 20:55 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Theodore Tso, git

Stephen Boyd <bebarino@gmail.com> writes:

> On Wed, Jun 16, 2010 at 9:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Theodore Tso <tytso@MIT.EDU> writes:
>>
>>> I don't know how important people will feel this to be, but I've occasionally
>>> found it interesting to see how many people are using various different
>>> versions of git in a particular development community, and having the
>>> version in the signature is a useful gauge on that.
>>
>> The original motivation of the version signature was exactly that ;-).
> ...
> The real question is how useful is it to have the version number
> there? There are other ways to gauge the version usage and it's opt-in
> so you'll still get some version number coverage.

I sent the previous message without finishing that sentence.  I personally
have been feeling that the version number there outlived its usefulness
for quite some time.

If others (like Ted) find it useful/amusing/interesting, we may want to
keep the default and make sure it is not too easy to customize it away,
and I think what your patch does is fine.

Thanks.

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

end of thread, other threads:[~2010-06-16 20:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-15  5:00 [PATCH 1/2] format-patch: Add a signature option (--signature) Stephen Boyd
2010-06-15  5:00 ` [PATCH 2/2] completion: Add --signoff and format.signoff Stephen Boyd
2010-06-15 17:05 ` [PATCH 1/2] format-patch: Add a signature option (--signature) Junio C Hamano
2010-06-16  4:31   ` Stephen Boyd
2010-06-16  5:59 ` [PATCHv2 " Stephen Boyd
2010-06-16  5:59 ` [PATCHv2 2/2] completion: Add --signature and format.signature Stephen Boyd
2010-06-16 13:13 ` [PATCH 1/2] format-patch: Add a signature option (--signature) Theodore Tso
2010-06-16 16:53   ` Junio C Hamano
2010-06-16 19:43     ` Stephen Boyd
2010-06-16 20:55       ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).