Re: [PATCH 01/10] Add a birdview-on-the-source-code section to the user manual

Jeff King [off-list ref] writes:

Unfortunately, I don't think we have the encoding information any more
at that point. We can infer how the patch was generated by looking at
the git-config, and that should be right 99% of the time (unless the
patches were generated with a different config, either from another repo
or before some settings were changed).

Junio, can you confirm my understanding that:
  - if i18n.logOutputEncoding is set, then we are definitely in that
    encoding
  - otherwise, if i18n.commitEncoding is set, we should assume commits are
    in that encoding (which is just a guess, since they may have been
    generated on another config, but it's our best guess)
  - otherwise, assume utf-8

I do not want to break projects whose members consistently use a
single non UTF-8 encoding, and I've been hoping that in such a
use case they should not have to set any of these encoding
configuration.  So in that sense I would be somewhat reluctant
to agree with the last one.  But I am getting a feeling that it
is a losing battle.

On the patch acceptance side, when we do _not_ have encoding
information and the input does not look like a valid UTF-8, we
assume that the input is latin-1 and convert it to UTF-8, if I
recall correctly.  If somebody sent you a patch without encoding
header, and then you are forwarding that patch, not adding
anything ourselves (because we do not know) and let the
receiving end to do that conversion is certainly the best; but
if we _were_ to add anything I would suspect it would be a
better idea to use the same logic to default to latin-1 or
UTF-8.  East Asian users may want to raise objections here.

I think it is a reasonable compromise to do it the way you
outlined.  Doing it at patch generation time would fix the
ambiguity issues during the step 2, so it might turn out to be
necessary to add the encoding header to format-patch output
after all, but send-email needs to be able to handle messages
that do not have the header anyway, so probably the first step
is to do so in send-email.

When we update format-patch, the ambiguity at step 2 would
disappear.  My gut feeling is that adding an extra header to
format-patch output would not break people's workflow nor
scripts (I do not think it would break mine, as I either suck in
only the body of the message to my MUA or use send-email), but I
am not sure.

Also Junio, it looks like commit 7cbcf4d5 moved parsing of the
--encoding parameter into setup_revisions, but it's still being checked
for in cmd_log_init. Can you confirm that the latter is now superfluous
and can be removed?

Thanks for noticing, and I think you are right.  The code parses
the same input and sets the same global variable the same way.

On Tue, May 15, 2007 at 11:41:01AM -0700, Junio C Hamano wrote:

I do not want to break projects whose members consistently use a
single non UTF-8 encoding, and I've been hoping that in such a
use case they should not have to set any of these encoding
configuration.  So in that sense I would be somewhat reluctant
to agree with the last one.  But I am getting a feeling that it
is a losing battle.

I think that is a good goal, but I think we have already failed, as
git-format-patch generates content-type headers with charset=utf-8
(unless the encoding variables are set up). This code was added last
year around this time (cdd406e38).

It looks like this is squelched in the presence of format.headers
configuration. However, that still means they have to do _something_ to
get it to work right (and I note that the fact that format.headers
squelches MIME headers doesn't seem to be documented anywhere...)

I think it is a reasonable compromise to do it the way you
outlined.  Doing it at patch generation time would fix the
ambiguity issues during the step 2, so it might turn out to be
necessary to add the encoding header to format-patch output
after all, but send-email needs to be able to handle messages
that do not have the header anyway, so probably the first step
is to do so in send-email.

As I noted in my other email, it actually _is_ there already. So the
MIME-Version fix just keeps the status quo, and we've been doing it this
way for a year.

Is it still worth making these guesses in send-email?

quoted

Also Junio, it looks like commit 7cbcf4d5 moved parsing of the
--encoding parameter into setup_revisions, but it's still being checked
for in cmd_log_init. Can you confirm that the latter is now superfluous
and can be removed?

Thanks for noticing, and I think you are right.  The code parses
the same input and sets the same global variable the same way.

Well, I wouldn't have noticed it if you hadn't written git-log -S. :) In
case you haven't fixed it yet, here it is in patch form:

-- >8 --
cmd_log_init: remove parsing of --encoding command line parameter

This was moved to the setup_revisions parsing in 7cbcf4d5, so it was
never being triggered.

Signed-off-by: Jeff King <redacted>
---

diff --git a/builtin-log.c b/builtin-log.c
index 3744712..cebb958 100644
--- a/builtin-log.c
+++ b/builtin-log.c

@@ -60,13 +60,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		rev->always_show_header = 0;
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
-		if (!prefixcmp(arg, "--encoding=")) {
-			arg += 11;
-			if (strcmp(arg, "none"))
-				git_log_output_encoding = xstrdup(arg);
-			else
-				git_log_output_encoding = "";
-		} else if (!strcmp(arg, "--decorate")) {
+		if (!strcmp(arg, "--decorate")) {
 			if (!decorate)
 				for_each_ref(add_ref_decoration, NULL);
 			decorate = 1;