Re: [PATCH v8 2/2] format-patch --signature-file file
Jeremiah Mahler jmmah...@gmail.com writes: Your comments make it clear that I have not accounted for all the possible cases. Below is a table of all the reasonable cases. It should account for cases you mentioned as well as others. Key: --- default: Git version number sig1: .signature from column 1 file1: .signaturefile from column 1 sig2: --signature from column 2 file2: --signature-file in column 2 A preceeding 'format.' is assumed for config. .signature - format.signature Command line arguments take precedence over config options. ;-) I do not mind the full matrix if it is cleanly done. It may be an overkill and a maintenance burden, especially if it can be done only with a sequence of cut-and-paste repetition (i.e. not as a loop in the test script that synthesises the permutations). The suggestion for those three combinations were meant to be an easy-to-implement-and-maintain compromise. Thanks. |+-+-| | config (1)| argv (2) | | |+-+-| || | default | || --signature | sig2| || --signature-file| file2 | || --no-signature | none| || --no-signature-file | none| || --signature, --signature-file | die | || --signature, --no-signature-file| sig2| || --signature-file, --no-signature| none| || --no-signature, --no-signature-file | none| | .signature | | sig1| | .signature | --signature | sig2| | .signature | --signature-file| file2 | | .signature | --no-signature | none| | .signature | --no-signature-file | sig1| | .signature | --signature, --signature-file | die | | .signature | --signature, --no-signature-file| sig2| | .signature | --signature-file, --no-signature| none| | .signature | --no-signature, --no-signature-file | none| | .signaturefile | | file1 | | .signaturefile | --signature | sig2| | .signaturefile | --signature-file| file2 | | .signaturefile | --no-signature | none| | .signaturefile | --no-signature-file | default | | .signaturefile | --signature, --signature-file | die | | .signaturefile | --signature, --no-signature-file| sig2| | .signaturefile | --signature-file, --no-signature| none| | .signaturefile | --no-signature, --no-signature-file | none| | .signature, .signaturefile | | die | | .signature, .signaturefile | --signature | sig2| | .signature, .signaturefile | --signature-file| file2 | | .signature, .signaturefile | --no-signature | none| | .signature, .signaturefile | --no-signature-file | sig1| | .signature, .signaturefile | --signature, --signature-file | die | | .signature, .signaturefile | --signature, --no-signature-file| sig2| | .signature, .signaturefile | --signature-file, --no-signature| none| | .signature, .signaturefile | --no-signature, --no-signature-file | none| |+-+-| Thanks, -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/2] format-patch --signature-file file
Jeremiah Mahler jmmah...@gmail.com writes: Added option that allows a signature file to be used with format-patch so that signatures with newlines and other special characters can be easily included. s/Added option/Add an option/. I do not think with newlines and other special characters is the primary issue---isn't it more about I have chosen to use this mail-signature; do not force me to retype the same all the time? $ git format-patch --signature-file ~/.signature -1 The recommended command-line convention (see gitcli(7)) is to use --option=value, so an example would be better to follow it, i.e. $ git format-patch -1 --signature-file=$HOME/.signature The config variable format.signaturefile is also provided so that it can be added by default. $ git config format.signaturefile ~/.signature $ git format-patch -1 Something like: To countermand the configuration variable for a specific run: $ git format-patch -1 --signature=This time only $ git format-patch -1 --signature;# to use the default $ git format-patch -1 --signature= ;# to add nothing is also needed here, I think. Similarly, these two needs to be tested in the test scripts you are modifying. Specifically: +test_expect_success 'format-patch --no-signature and --signature-file OK' ' + git format-patch --stdout --no-signature --signature-file=mail-signature -1 +' should not just make sure format-patch does _something_, but needs to make sure it does not contain the contents of the configured mail signagture file. I didn't see offhand if the tests make sure that a configured mail signature can be overriden from the command line. I think you would want to test, with format-patch.signature-file pointing at the mail-signature file, at least these three cases: - Run format-patch --no-signature and make sure that stops the contents from mail-signature file from being shown, and instead no mail-signature is given. - Run format-patch --signature='this time only' and make sure that stops the contents from mail-signature file from being shown and this time only is used instead. - Run format-patch --signature-file=another-mail-signature and make sure that stops the contents from mail-signature file from being shown and the contents from the other file is used instead. Test for these negative cases is often what we forget, when we are thrilled to show off that the shiny new feature works as expected. We need to ensure that the ways to stop the shiny new feature from kicking in will not be broken as well. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 2/2] format-patch --signature-file file
Added option that allows a signature file to be used with format-patch
so that signatures with newlines and other special characters can be
easily included.
$ git format-patch --signature-file ~/.signature -1
The config variable format.signaturefile is also provided so that it
can be added by default.
$ git config format.signaturefile ~/.signature
$ git format-patch -1
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
Documentation/config.txt | 4
Documentation/git-format-patch.txt | 4
builtin/log.c | 16 +++
t/t4014-format-patch.sh| 41 ++
4 files changed, 65 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1932e9b..140ed77 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1114,6 +1114,10 @@ format.signature::
Set this variable to the empty string () to suppress
signature generation.
+format.signaturefile::
+ Works just like format.signature except the contents of the
+ file specified by this variable will be used as the 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 5c0a4ab..c0fd470 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -14,6 +14,7 @@ SYNOPSIS
[(--attach|--inline)[=boundary] | --no-attach]
[-s | --signoff]
[--signature=signature | --no-signature]
+ [--signature-file=file]
[-n | --numbered | -N | --no-numbered]
[--start-number n] [--numbered-files]
[--in-reply-to=Message-Id] [--suffix=.sfx]
@@ -233,6 +234,9 @@ configuration options in linkgit:git-notes[1] to use this
workflow).
signature option is omitted the signature defaults to the Git version
number.
+--signature-file=file::
+ Works just like --signature except the signature is read from a file.
+
--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 5acc048..5e3cc29 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -673,6 +673,7 @@ static void add_header(const char *value)
static int thread;
static int do_signoff;
static const char *signature = git_version_string;
+static const char *signature_file;
static int config_cover_letter;
enum {
@@ -742,6 +743,8 @@ static int git_format_config(const char *var, const char
*value, void *cb)
}
if (!strcmp(var, format.signature))
return git_config_string(signature, var, value);
+ if (!strcmp(var, format.signaturefile))
+ return git_config_pathname(signature_file, var, value);
if (!strcmp(var, format.coverletter)) {
if (value !strcasecmp(value, auto)) {
config_cover_letter = COVER_AUTO;
@@ -1235,6 +1238,8 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
PARSE_OPT_OPTARG, thread_callback },
OPT_STRING(0, signature, signature, N_(signature),
N_(add a signature)),
+ OPT_FILENAME(0, signature-file, signature_file,
+ N_(add a signature from a file)),
OPT__QUIET(quiet, N_(don't print the patch filenames)),
OPT_END()
};
@@ -1452,6 +1457,17 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
cover_letter = (config_cover_letter == COVER_ON);
}
+ if (signature_file) {
+ struct strbuf buf = STRBUF_INIT;
+
+ if (signature signature != git_version_string)
+ die(_(cannot specify both signature and
signature-file));
+
+ if (strbuf_read_file(buf, signature_file, 128) 0)
+ die_errno(_(unable to read signature file '%s'),
signature_file);
+ signature = strbuf_detach(buf, NULL);
+ }
+
if (in_reply_to || thread || cover_letter)
rev.ref_message_ids = xcalloc(1, sizeof(struct string_list));
if (in_reply_to) {
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 9c80633..37d25c4 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -762,6 +762,47 @@ test_expect_success 'format-patch --signature=
suppresses signatures' '
! grep ^-- \$ output
'
+test_expect_success 'prepare mail-signature input' '
+ cat mail-signature -\EOF
+
+ Test User test.em...@kernel.org
+ http://git.kernel.org/cgit/git/git.git
+
+
