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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 9, 2011, 14:11 UTC
Message-ID: <7vy5vpv11n.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1320836458-24088-1-git-send-email-Alex.Crezoff@gmail.com>

Alexey Shumkin <alex.crezoff@gmail.com> writes:

Show 16 quoted lines

> 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?

> Signed-off-by: Alexey Shumkin <Alex.Crezoff@gmail.com>
> ---

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.

Show 12 quoted lines

> 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?

> +# 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?

Show 28 quoted lines

> @@ -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?

It almost seems backwards; converting the payload to UTF-8 and always sending UTF-8 would be a simpler approach, methinks.