Re: [PATCH v3] post-receive-email: explicitly set Content-Type header

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> Alexey Shumkin <alex.crezoff@xxxxxxxxx> writes:
> 
> > Some email clients (e.g. claws-mail) incorrectly display
> > message body when there is no Content-Type header and charset
> > explicitly defined.
> > So, set explicitly Content-Type header. Its charset
> > can be defined with hooks.emailcharset config variable.
> >
> > NB: This above-mentioned charset may differ from
> > i18n.logOutputEncoding, because e.g. gitweb expects (for now)
> > i18n.logOutputEncoding set to UTF-8 to display logs correctly.
> >
> > Also, introduce hooks.gitopts config variable
> > with the default '-c core.quotepath=false'.
> > This takes into account that we want to see pretty email-message
> > with well-looking messages and list of changed filenames.
> > And usually non-ASCII filenames are in the same
> > encoding that commit messages are.
> 
> (style) Why such an extremely ragged looking line-wrap of paragraphs?
I'm not good enough in English spelling ;(
> 
> > Signed-off-by: Alexey Shumkin <Alex.Crezoff@xxxxxxxxx>
> > ---
> 
> In this space, please describe what happened during v1 and v2, and
> how is this round different to help reviewers. Pointers to list
> archive, e.g.
> http://thread.gmane.org/gmane.comp.version-control.git/181737, would
> be helpful.
> 
> People involved in v1/v2 discussion are missing from the Cc: line.
> Please do not give a false impression that you are hiding from them.
Oh, I've missed that moment, sorry all
> 
> > diff --git a/contrib/hooks/post-receive-email
> > b/contrib/hooks/post-receive-email index ba077c1..913be89 100755
> > --- a/contrib/hooks/post-receive-email
> > +++ b/contrib/hooks/post-receive-email
> > @@ -65,6 +65,14 @@
> >  #   Default is "--stat --summary --find-copies-harder". Add -p to
> > those #   options to include a unified diff of changes in addition
> > to the usual #   summary output.
> > +# hooks.gitopts
> > +#   git options for the git diff-tree invocation that shows
> > changes. +#   Default is '-c core.quotepath=false' to be able to
> > see non-ASCII filenames +#   used in a project.
> 
> We do not particularly appreciate a patch that does two unrelated
> things ("they are both related to post-receive-email" is not an
> argument). Wouldn't this be useful even if the change to add
> hooks.emailcharset turned out to be unwanted, or vice versa?
The main reason was that using core.quotepath=false leads to showing
file names "correctly" according to commit messages encoding to make
email-message look pretty.
> 
> > +# hooks.emailcharset
> > +#   The charset used in Content-Type header. UTF-8, if not
> > specified. +#   It can differ from i18n.logOutputEncoding (not to
> > mess-up with gitweb +#   which expects i18n.logOutputEncoding to be
> > set to UTF-8)
> 
> Why "UTF-8" instead of "i18n.logoutputencoding" if not specified?
Well, you're right.
For the explanation:
AFAIU, such hooks are used on central servers to notify involved people
about changes. And AFAIU the same server repos are used with gitweb
(which AFAIK requires i18n.logOutputEncoding=UTF-8)

But in common case "i18n.logoutputencoding" is more suitable.

> 
> > @@ -234,6 +242,9 @@ generate_email_header()
> >  	cat <<-EOF
> >  	To: $recipients
> >  	Subject: ${emailprefix}$projectdesc $refname_type
> > $short_refname ${change_type}d. $describe
> > +	MIME-Version: 1.0
> > +	Content-Type: text/plain; charset=$emailcharset
> > +	Content-Transfer-Encoding: 8bit
> >  	X-Git-Refname: $refname
> >  	X-Git-Reftype: $refname_type
> >  	X-Git-Oldrev: $oldrev
> > ...
> > @@ -730,6 +734,19 @@ custom_showrev=$(git config hooks.showrev)
> >  maxlines=$(git config hooks.emailmaxlines)
> >  diffopts=$(git config hooks.diffopts)
> >  : ${diffopts:="--stat --summary --find-copies-harder"}
> > +gitopts=$(git config hooks.gitopts || echo '-c
> > core.quotepath=false') +emailcharset=$(git config
> > hooks.emailcharset || echo 'UTF-8') +
> > +projectdesc=$(sed -ne '1p' "$GIT_DIR/description" 2>/dev/null)
> > +# Check if the description is unchanged from it's default, and
> > shorten it to +# a more manageable length if it is
> > +if expr "$projectdesc" : "Unnamed repository.*$" >/dev/null
> > +then
> > +	projectdesc="UNNAMED PROJECT"
> > +fi
> > +# Leave description in UTF-8 to be used in the Subject header
> > +# But convert it to an hooks.emailcharset encoding to be used in a
> > message body +projectdesc_e=$(echo $projectdesc | iconv -f UTF-8 -t
> > $emailcharset 2>/dev/null)
> 
> Hmm, this generates a piece of e-mail whose subject line is in UTF-8
> (without B/Q quoting) and message body is in totally different
> encoding. Is it what mailers really want to see?
Here you're right, too. Windows email clients may interpret Subject
header without B/Q quoting in its default Windows charset, and as far as
it may contain non-English project description, so Subject would look
ugly. But I'll try to test with some clients.
> 
> It almost seems backwards; converting the payload to UTF-8 and always
> sending UTF-8 would be a simpler approach, methinks.
Sounds reasonable

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]