From: Mike Frysinger <vapier@gentoo.org>
Date: Thu, 21 Jan 2010 17:59:29 GMT
Subject: Re: [PATCH] post-receive-email: allow customizing of subject/intro/footer
Message-ID: <201001211259.30704.vapier@gentoo.org>
In-Reply-To: <4B5888C2.60508@xiplink.com>

On Thursday 21 January 2010 12:02:58 Marc Branchaud wrote:
> > +	if [ -n "${emailbodyintro}" ] ; then
> > +		printf '\n%s\n' "${emailbodyintro}"
> > +	fi
> 
> Since the script ensures there's always a value set for emailbodyintro, why
> check for it here?  None of the other uses of the new config items
> (emailsubject and emailfooter) have this kind of check.

the other values dont have newlines added to them implicitly.  if it's empty, 
i dont want to useless newlines at the start of the e-mail.

> > -	The $refname_type, $short_refname has been ${change_type}d
> > -	EOF
> > +	printf '\n%s\n' "The @refname_type@, @short_refname@ has been
> > @change_type@d" +	) | subst_vars
> 
> Any reason why that last printf'd line shouldn't be made part of the
> emailbodyintro?

that could work

> So, overall, why not make generate_email_header() be simply:
> 
> generate_email_header()
> {
> 	# --- Email (all stdout will be the email)
> 	# Generate header
> 	subst_vars <<-EOF
> 	To: $recipients
> 	Subject: ${emailprefix}${emailsubject}
> 	X-Git-Refname: @refname@
> 	X-Git-Reftype: @refname_type@
> 	X-Git-Oldrev: @oldrev@
> 	X-Git-Newrev: @newrev@
> 
> 	${emailbodyintro}
> 
> 	EOF
> }
> 
> This would also let you simply subst_vars() so that it needn't support
>  piped invocations, no?  (Not a very drastic simplification, but still...)

if emailbodyintro is empty, this adds two useless newlines.  otherwise, this 
would be fine i think.
-mike