From: Junio C Hamano <junkio@cox.net>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org, junkio@cox.net
Subject: Re: [PATCH] commit encoding: store it in commit header rather than mucking with NUL
Date: Sun, 24 Dec 2006 16:13:42 -0800 [thread overview]
Message-ID: <7v3b74q1c9.fsf@assigned-by-dhcp.cox.net> (raw)
In-Reply-To: <Pine.LNX.4.63.0612241643440.19693@wbgn013.biozentrum.uni-wuerzburg.de> (Johannes Schindelin's message of "Sun, 24 Dec 2006 16:44:50 +0100 (CET)")
Thanks for paying a very close attention to what is in 'pu'.
The series was a work in progress not yet ready for publication,
which I merged by mistake because I was way too sleepy.
There are three reasons (and a half -- it was not even compile
tested) that I did not mean to merge it yet.
- I did not want to break existing git implementations, but
hadn't audited the commit header parsers to see if they do
not get upset when they see unrecognized header fields in
commit objects. The 'trailer' was just a quick hack to have
something working for me to test the output conversion code.
I've looked at the code since then and I think it is Ok to
add new ones after author/name fields like you did.
If we _were_ to do this, I think it is preferable to make it
a header, not a trailer like I did.
- I was not sure if the "assume the whole commit->buffer is in
the local encoding and recode it into UTF-8" is correct. The
message body ought to be in i18n.commitencoding from the
repository the commit comes from, which is now recorded in
the trailer (or the header as in your patch), but what I was
unsure about was the encoding in which author and committer
names are recorded. I think they are set either by the
environment variables or user.name to be consistent with
i18n.commitencoding in the repository the commit is made, so
it is Ok after all.
- Existing Porcelains such as gitk know i18n.commitencoding is
a hint to them by the core, and expect the core to give them
output in the local encoding. With the change, the core
feeds UTF-8 to the caller, unless the Porcelain gets the log
with plumbing "cat-file". This means they either have to
lose code to do their own recoding (which is probably a good
thing in the long run), or we would need to have a flag for
them to tell the core not to do the conversion. But a new
flag to ask for older behaviour is always a wrong way of
transitioning across backward incompatibility.
I think the output conversion from the log should be more
explicitly asked for it, than just a mere configuration
variable that cannot be overriden by gitk and friends.
next prev parent reply other threads:[~2006-12-25 0:13 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-12-24 14:18 i18n.logToUTF8: convert commit log message to UTF-8 Johannes Schindelin
2006-12-24 15:44 ` [PATCH] commit encoding: store it in commit header rather than mucking with NUL Johannes Schindelin
2006-12-25 0:13 ` Junio C Hamano [this message]
2006-12-25 0:41 ` Johannes Schindelin
2006-12-25 7:27 ` Junio C Hamano
2006-12-25 22:54 ` Jakub Narebski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7v3b74q1c9.fsf@assigned-by-dhcp.cox.net \
--to=junkio@cox.net \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).