Re: [PATCH v3] format-patch: Add --rfc for the common case of [RFC PATCH]

2016-09-21 Thread Junio C Hamano
Josh Triplett  writes:

> This provides a shorter and more convenient alias for
> --subject-prefix='RFC PATCH'.

Shorter and more convenient is quite subjective but more important
as a justification is that we believe [RFC PATCH] is used fairly
commonly (at least in certain circles).

> Includes documentation in the format-patch manpage, and a new test
> covering --rfc.

We can see that from diffstat ;-)

I'd retitle this like so:

format-patch: add "--rfc" for the common case of [RFC PATCH]

Add an alias for --subject-prefix='RFC PATCH', which is used
commonly in some development communities to deserve such a
short-hand.

Signed-off-by: Josh Triplett 
Reviewed-by: Jeff King 
Signed-off-by: Junio C Hamano 

> diff --git a/Documentation/git-format-patch.txt 
> b/Documentation/git-format-patch.txt
> index 9624c84..9b200b3 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -19,7 +19,8 @@ SYNOPSIS
>  [--start-number ] [--numbered-files]
>  [--in-reply-to=Message-Id] [--suffix=.]
>  [--ignore-if-in-upstream]
> -[--subject-prefix=Subject-Prefix] [(--reroll-count|-v) ]
> +[--rfc] [--subject-prefix=Subject-Prefix]
> +[(--reroll-count|-v) ]
>  [--to=] [--cc=]
>  [--[no-]cover-letter] [--quiet] [--notes[=]]
>  []
> @@ -172,6 +173,11 @@ will want to ensure that threading is disabled for `git 
> send-email`.
>   allows for useful naming of a patch series, and can be
>   combined with the `--numbered` option.
>  
> +--rfc::
> + Alias for `--subject-prefix="RFC PATCH"`. RFC means "Request For
> + Comments"; use this when sending an experimental patch for
> + discussion rather than application.
> +

I do not think we want to be in the business of encouragign or
discouraging the use of "[RFC PATCH]".  

--rfc:: A short-hand for `--subject-prefix="RFC PATCH"`.
RFC stands for "request for comments" and such a
prefix is used in some development communities when
sending a patch primarily to illustrate an idea to
help discussion, rather than to be applied.

perhaps?

The code and test both look good to me.

Thanks.


Re: [PATCH v3] format-patch: Add --rfc for the common case of [RFC PATCH]

2016-09-19 Thread Jeff King
On Mon, Sep 19, 2016 at 09:23:25PM -0700, Josh Triplett wrote:

> This provides a shorter and more convenient alias for
> --subject-prefix='RFC PATCH'.
> 
> Includes documentation in the format-patch manpage, and a new test
> covering --rfc.
> 
> Signed-off-by: Josh Triplett 
> ---
> v3:
> - Fix an error message referring to --subject-prefix
> - Expand the acronym "RFC"

This looks fine to me. Thanks.

-Peff


[PATCH v3] format-patch: Add --rfc for the common case of [RFC PATCH]

2016-09-19 Thread Josh Triplett
This provides a shorter and more convenient alias for
--subject-prefix='RFC PATCH'.

Includes documentation in the format-patch manpage, and a new test
covering --rfc.

Signed-off-by: Josh Triplett 
---
v3:
- Fix an error message referring to --subject-prefix
- Expand the acronym "RFC"
v2:
- Add documentation to the format-patch manpage
- Call subject_prefix_callback rather than reimplementing it
- Update test to move expectations inside

 Documentation/git-format-patch.txt |  8 +++-
 builtin/log.c  | 10 +-
 t/t4014-format-patch.sh|  9 +
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-format-patch.txt 
b/Documentation/git-format-patch.txt
index 9624c84..9b200b3 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -19,7 +19,8 @@ SYNOPSIS
   [--start-number ] [--numbered-files]
   [--in-reply-to=Message-Id] [--suffix=.]
   [--ignore-if-in-upstream]
-  [--subject-prefix=Subject-Prefix] [(--reroll-count|-v) ]
+  [--rfc] [--subject-prefix=Subject-Prefix]
+  [(--reroll-count|-v) ]
   [--to=] [--cc=]
   [--[no-]cover-letter] [--quiet] [--notes[=]]
   []
@@ -172,6 +173,11 @@ will want to ensure that threading is disabled for `git 
send-email`.
allows for useful naming of a patch series, and can be
combined with the `--numbered` option.
 
+--rfc::
+   Alias for `--subject-prefix="RFC PATCH"`. RFC means "Request For
+   Comments"; use this when sending an experimental patch for
+   discussion rather than application.
+
 -v ::
 --reroll-count=::
Mark the series as the -th iteration of the topic. The
diff --git a/builtin/log.c b/builtin/log.c
index 92dc34d..c657900 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1112,6 +1112,11 @@ static int subject_prefix_callback(const struct option 
*opt, const char *arg,
return 0;
 }
 
+static int rfc_callback(const struct option *opt, const char *arg, int unset)
+{
+   return subject_prefix_callback(opt, "RFC PATCH", unset);
+}
+
 static int numbered_cmdline_opt = 0;
 
 static int numbered_callback(const struct option *opt, const char *arg,
@@ -1419,6 +1424,9 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
N_("start numbering patches at  instead of 1")),
OPT_INTEGER('v', "reroll-count", _count,
N_("mark the series as Nth re-roll")),
+   { OPTION_CALLBACK, 0, "rfc", , NULL,
+   N_("Use [RFC PATCH] instead of [PATCH]"),
+   PARSE_OPT_NOARG | PARSE_OPT_NONEG, rfc_callback },
{ OPTION_CALLBACK, 0, "subject-prefix", , N_("prefix"),
N_("Use [] instead of [PATCH]"),
PARSE_OPT_NONEG, subject_prefix_callback },
@@ -1557,7 +1565,7 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
if (numbered && keep_subject)
die (_("-n and -k are mutually exclusive."));
if (keep_subject && subject_prefix)
-   die (_("--subject-prefix and -k are mutually exclusive."));
+   die (_("--subject-prefix/--rfc and -k are mutually 
exclusive."));
rev.preserve_subject = keep_subject;
 
argc = setup_revisions(argc, argv, , _r_opt);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index b0579dd..ed4d3c2 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1073,6 +1073,15 @@ test_expect_success 'empty subject prefix does not have 
extra space' '
test_cmp expect actual
 '
 
+test_expect_success '--rfc' '
+   cat >expect <<-\EOF &&
+   Subject: [RFC PATCH 1/1] header with . in it
+   EOF
+   git format-patch -n -1 --stdout --rfc >patch &&
+   grep ^Subject: patch >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success '--from=ident notices bogus ident' '
test_must_fail git format-patch -1 --stdout --from=foo >patch
 '

base-commit: 6ebdac1bab966b720d776aa43ca188fe378b1f4b
-- 
git-series 0.8.10