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
Attachment:
signature.asc
Description: This is a digitally signed message part.