On 12:27 Wed 16 Jun , Junio C Hamano wrote:
> Zhang Le <r0bertz@xxxxxxxxxx> 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:
pgpLGFo28Ottm.pgp
Description: PGP signature