[PATCH v3 1/2] add --recode-patch option to git-mailinfo

[PATCH v3 1/2] add --recode-patch option to git-mailinfo

[Zhang Le]

I have a translation project which uses UTF-8 as charset.
So the patch must be encoded in UTF-8, not just the commit msg etc.
And we use google group as our mailing list.

Recently, mails saved from gmail are encoded using local charset based on
google group's setting. This seems smart but it caused inconvenience for
our project.

Since we have no control on what google will do, so I took another way,
i.e. add this option to git-mailinfo. I hope this could benefit others as
well.

Changelog:

v2 -> v3:
Removed 'const' type qualifier from handle_patch()'s parameter
Fixed typos in commit msg

v1 -> v2:
Clarified how -u/--encoding is handled in git-mailinfo's documentation

Signed-off-by: Zhang Le <r0bertz@gentoo.org>
---
 Documentation/git-mailinfo.txt |    6 +++++-
 builtin/mailinfo.c             |   10 ++++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-mailinfo.txt b/Documentation/git-mailinfo.txt
index 3ea5aad..24d5bd7 100644
--- a/Documentation/git-mailinfo.txt
+++ b/Documentation/git-mailinfo.txt
@@ -45,7 +45,7 @@ OPTIONS
 	them.  This used to be optional but now it is the default.
 +
 Note that the patch is always used as-is without charset
-conversion, even with this flag.
+conversion, even with this flag.  Unless --recode-patch is used.
 
 --encoding=<encoding>::
 	Similar to -u.  But when re-coding, the charset specified here is
@@ -54,6 +54,10 @@ conversion, even with this flag.
 -n::
 	Disable all charset re-coding of the metadata.
 
+--recode-patch::
+	Similar to -u.  But what is re-coded is the patch instead of the
+	metainfo.  The default is off.
+
 --scissors::
 	Remove everything in body before a scissors line.  A line that
 	mainly consists of scissors (either ">8" or "8<") and perforation
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 4a9729b..bb87b32 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -12,6 +12,7 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
 static int keep_subject;
 static int keep_non_patch_brackets_in_subject;
 static const char *metainfo_charset;
+static int recode_patch;
 static struct strbuf line = STRBUF_INIT;
 static struct strbuf name = STRBUF_INIT;
 static struct strbuf email = STRBUF_INIT;
@@ -828,8 +829,10 @@ static int handle_commit_msg(struct strbuf *line)
 	return 0;
 }
 
-static void handle_patch(const struct strbuf *line)
+static void handle_patch(struct strbuf *line)
 {
+	if (recode_patch)
+		convert_to_utf8(line, charset.buf);
 	fwrite(line->buf, 1, line->len, patchfile);
 	patch_lines++;
 }
@@ -1021,7 +1024,7 @@ static int git_mailinfo_config(const char *var, const char *value, void *unused)
 }
 
 static const char mailinfo_usage[] =
-	"git mailinfo [-k|-b] [-u | --encoding=<encoding> | -n] [--scissors | --no-scissors] msg patch < mail >info";
+	"git mailinfo [-k|-b] [-u | --encoding=<encoding> | -n] [--recode-patch] [--scissors | --no-scissors] msg patch < mail >info";
 
 int cmd_mailinfo(int argc, const char **argv, const char *prefix)
 {
@@ -1034,6 +1037,7 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)
 
 	def_charset = (git_commit_encoding ? git_commit_encoding : "UTF-8");
 	metainfo_charset = def_charset;
+	recode_patch = 0;
 
 	while (1 < argc && argv[1][0] == '-') {
 		if (!strcmp(argv[1], "-k"))
@@ -1046,6 +1050,8 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)
 			metainfo_charset = NULL;
 		else if (!prefixcmp(argv[1], "--encoding="))
 			metainfo_charset = argv[1] + 11;
+		else if (!strcmp(argv[1], "--recode-patch"))
+			recode_patch = 1;
 		else if (!strcmp(argv[1], "--scissors"))
 			use_scissors = 1;
 		else if (!strcmp(argv[1], "--no-scissors"))
-- 
1.7.1

[PATCH v3 2/2] add --recode-patch option to git-am

[Zhang Le]

The reason has been explained in the "add --recode-patch to git-mailinfo" patch

Signed-off-by: Zhang Le <r0bertz@gentoo.org>
---
 Documentation/git-am.txt |    4 ++++
 git-am.sh                |   13 +++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 9e62f87..38c1442 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -73,6 +73,10 @@ default.   You can use `--no-utf8` to override this.
 	Pass `-n` flag to 'git mailinfo' (see
 	linkgit:git-mailinfo[1]).
 
+--recode-patch::
+	Pass `--recode-patch` flag to 'git mailinfo' (see
+	linkgit:git-mailinfo[1]).
+
 -3::
 --3way::
 	When the patch does not apply cleanly, fall back on
diff --git a/git-am.sh b/git-am.sh
index 87ffae2..49cf788 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -14,6 +14,7 @@ b,binary*       (historical option -- no-op)
 q,quiet         be quiet
 s,signoff       add a Signed-off-by line to the commit message
 u,utf8          recode into utf8 (default)
+recode-patch    pass --recode-patch flag to git-mailinfo
 k,keep          pass -k flag to git-mailinfo
 keep-cr         pass --keep-cr flag to git-mailsplit for mbox format
 no-keep-cr      do not pass --keep-cr flag to git-mailsplit independent of am.keepcr
@@ -294,7 +295,7 @@ split_patches () {
 prec=4
 dotest="$GIT_DIR/rebase-apply"
 sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors= no_inbody_headers=
+resolvemsg= resume= scissors= no_inbody_headers= recode_patch=
 git_apply_opt=
 committer_date_is_author_date=
 ignore_date=
@@ -320,6 +321,8 @@ do
 		utf8=t ;; # this is now default
 	--no-utf8)
 		utf8= ;;
+	--recode-patch)
+		recode_patch=t ;;
 	-k|--keep)
 		keep=t ;;
 	-c|--scissors)
@@ -463,6 +466,7 @@ else
 	echo "$threeway" >"$dotest/threeway"
 	echo "$sign" >"$dotest/sign"
 	echo "$utf8" >"$dotest/utf8"
+	echo "$recode_patch" >"$dotest/recode_patch"
 	echo "$keep" >"$dotest/keep"
 	echo "$keepcr" >"$dotest/keepcr"
 	echo "$scissors" >"$dotest/scissors"
@@ -504,6 +508,10 @@ then
 else
 	utf8=-n
 fi
+if test "$(cat "$dotest/recode_patch")" = t
+then
+	recodepatch=--recode-patch
+fi
 if test "$(cat "$dotest/keep")" = t
 then
 	keep=-k
@@ -580,7 +588,8 @@ do
 	# by the user, or the user can tell us to do so by --resolved flag.
 	case "$resume" in
 	'')
-		git mailinfo $keep $no_inbody_headers $scissors $utf8 "$dotest/msg" "$dotest/patch" \
+		git mailinfo $keep $no_inbody_headers $scissors $utf8 \
+		$recodepatch "$dotest/msg" "$dotest/patch" \
 			<"$dotest/$msgnum" >"$dotest/info" ||
 			stop_here $this
 
-- 
1.7.1

Re: [PATCH v3 1/2] add --recode-patch option to git-mailinfo

[Junio C Hamano]

Zhang Le <r0bertz@gentoo.org> writes:

> diff --git a/Documentation/git-mailinfo.txt b/Documentation/git-mailinfo.txt
> index 3ea5aad..24d5bd7 100644
> --- a/Documentation/git-mailinfo.txt
> +++ b/Documentation/git-mailinfo.txt
> @@ -45,7 +45,7 @@ OPTIONS
>  	them.  This used to be optional but now it is the default.
>  +
>  Note that the patch is always used as-is without charset
> -conversion, even with this flag.
> +conversion, even with this flag.  Unless --recode-patch is used.

Somehow this doesn't rhyme well.  Perhaps

    Note that the patch is used as-with without charset conversion; use
    `--recode-patch` for that.

would be better?

> @@ -54,6 +54,10 @@ conversion, even with this flag.
>  -n::
>  	Disable all charset re-coding of the metadata.
>  
> +--recode-patch::
> +	Similar to -u.  But what is re-coded is the patch instead of the
> +	metainfo.  The default is off.

Ditto.

	Convert the patch from the e-mail to UTF-8 (or the value of the
	configuration variable `i18n.commitencoding`, if it is set).

By the way, what happens when somebody runs the following command?

	git mailinfo -n --recode-patch

Is it desirable?  If not, what _should_ happen instead?

> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
> index 4a9729b..bb87b32 100644
> --- a/builtin/mailinfo.c
> +++ b/builtin/mailinfo.c
> @@ -12,6 +12,7 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
>  static int keep_subject;
>  static int keep_non_patch_brackets_in_subject;
>  static const char *metainfo_charset;
> +static int recode_patch;
>  static struct strbuf line = STRBUF_INIT;
>  static struct strbuf name = STRBUF_INIT;
>  static struct strbuf email = STRBUF_INIT;
> @@ -828,8 +829,10 @@ static int handle_commit_msg(struct strbuf *line)
>  	return 0;
>  }
>  
> -static void handle_patch(const struct strbuf *line)
> +static void handle_patch(struct strbuf *line)
>  {
> +	if (recode_patch)
> +		convert_to_utf8(line, charset.buf);
>  	fwrite(line->buf, 1, line->len, patchfile);
>  	patch_lines++;
>  }
> @@ -1021,7 +1024,7 @@ static int git_mailinfo_config(const char *var, const char *value, void *unused)
>  }
>  
>  static const char mailinfo_usage[] =
> -	"git mailinfo [-k|-b] [-u | --encoding=<encoding> | -n] [--scissors | --no-scissors] msg patch < mail >info";
> +	"git mailinfo [-k|-b] [-u | --encoding=<encoding> | -n] [--recode-patch] [--scissors | --no-scissors] msg patch < mail >info";
>  
>  int cmd_mailinfo(int argc, const char **argv, const char *prefix)
>  {
> @@ -1034,6 +1037,7 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)
>  
>  	def_charset = (git_commit_encoding ? git_commit_encoding : "UTF-8");
>  	metainfo_charset = def_charset;
> +	recode_patch = 0;

Do you need this assignment?

Re: [PATCH v3 1/2] add --recode-patch option to git-mailinfo

[Zhang Le]

[-- Attachment #1: Type: text/plain, Size: 3646 bytes --]

On 12:27 Wed 16 Jun     , Junio C Hamano wrote:
> Zhang Le <r0bertz@gentoo.org> writes:
> 
> > diff --git a/Documentation/git-mailinfo.txt b/Documentation/git-mailinfo.txt
> > index 3ea5aad..24d5bd7 100644
> > --- a/Documentation/git-mailinfo.txt
> > +++ b/Documentation/git-mailinfo.txt
> > @@ -45,7 +45,7 @@ OPTIONS
> >  	them.  This used to be optional but now it is the default.
> >  +
> >  Note that the patch is always used as-is without charset
> > -conversion, even with this flag.
> > +conversion, even with this flag.  Unless --recode-patch is used.
> 
> Somehow this doesn't rhyme well.  Perhaps
> 
>     Note that the patch is used as-with without charset conversion; use
>     `--recode-patch` for that.
> 
> would be better?

OK

> 
> > @@ -54,6 +54,10 @@ conversion, even with this flag.
> >  -n::
> >  	Disable all charset re-coding of the metadata.
> >  
> > +--recode-patch::
> > +	Similar to -u.  But what is re-coded is the patch instead of the
> > +	metainfo.  The default is off.
> 
> Ditto.
> 
> 	Convert the patch from the e-mail to UTF-8 (or the value of the
> 	configuration variable `i18n.commitencoding`, if it is set).
> 

OK

> By the way, what happens when somebody runs the following command?
> 
> 	git mailinfo -n --recode-patch
> 
> Is it desirable?  If not, what _should_ happen instead?

This is not desirable. If -n is used, no recoding will happen.
Maybe we should warn users.
Maybe we use a separate variable like patch_charset, instead of reusing
metainfo_charset?

> 
> > diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
> > index 4a9729b..bb87b32 100644
> > --- a/builtin/mailinfo.c
> > +++ b/builtin/mailinfo.c
> > @@ -12,6 +12,7 @@ static FILE *cmitmsg, *patchfile, *fin, *fout;
> >  static int keep_subject;
> >  static int keep_non_patch_brackets_in_subject;
> >  static const char *metainfo_charset;
> > +static int recode_patch;
> >  static struct strbuf line = STRBUF_INIT;
> >  static struct strbuf name = STRBUF_INIT;
> >  static struct strbuf email = STRBUF_INIT;
> > @@ -828,8 +829,10 @@ static int handle_commit_msg(struct strbuf *line)
> >  	return 0;
> >  }
> >  
> > -static void handle_patch(const struct strbuf *line)
> > +static void handle_patch(struct strbuf *line)
> >  {
> > +	if (recode_patch)
> > +		convert_to_utf8(line, charset.buf);
> >  	fwrite(line->buf, 1, line->len, patchfile);
> >  	patch_lines++;
> >  }
> > @@ -1021,7 +1024,7 @@ static int git_mailinfo_config(const char *var, const char *value, void *unused)
> >  }
> >  
> >  static const char mailinfo_usage[] =
> > -	"git mailinfo [-k|-b] [-u | --encoding=<encoding> | -n] [--scissors | --no-scissors] msg patch < mail >info";
> > +	"git mailinfo [-k|-b] [-u | --encoding=<encoding> | -n] [--recode-patch] [--scissors | --no-scissors] msg patch < mail >info";
> >  
> >  int cmd_mailinfo(int argc, const char **argv, const char *prefix)
> >  {
> > @@ -1034,6 +1037,7 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)
> >  
> >  	def_charset = (git_commit_encoding ? git_commit_encoding : "UTF-8");
> >  	metainfo_charset = def_charset;
> > +	recode_patch = 0;
> 
> Do you need this assignment?

Of course, It is not required. Since variable in bss secton will be initialized
to 0. I just thought maybe it is better to initialized it explicitly, to make
it clear. Of course, this is only personal taste.

I will remove it, if that will suit git's coding style well. :)

-- 
Zhang, Le
Gentoo/Loongson Developer
http://zhangle.is-a-geek.org
0260 C902 B8F8 6506 6586 2B90 BC51 C808 1E4E 2973

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]