[PATCH v5] Re: git-am: allow e-mail file(s) as input

[Nicolas Sebrecht <nicolas.s.dev@gmx.fr>]

The 16/07/09, Nicolas Sebrecht wrote:
> The 16/07/09, Junio C Hamano wrote:
> > Stephen Boyd <bebarino@gmail.com> writes:
> > > Nicolas Sebrecht wrote:
> > 
> > >> +	# Then, accept what really looks like (series of) email(s).
> > >> +	# the first sed select headers but the folded ones
> > >> +	sed -e '/^$/q' -e '/^[[:blank:]]/d' "$1" |
> > >> +	# this one is necessary for the next 'grep -v'
> > >> +	sed -e '/^$/d' |
> > >> +	grep -v -E -e '^[A-Za-z]+(-[A-Za-z]+)*:' ||
> > >> +	{
> > >> +		patch_format=mbox
> > >> +		return 0
> > >> +	}
> > >> +
> > >>  	# otherwise, check the first few lines of the first patch to try
> > >>  	# to detect its format
> > >>  	{
> > >
> > > This fails t4150-am.sh #10 (am -3 -q is quiet). You should redirect the
> > > output of the sed and grep to /dev/null like Junio did in his "how about
> > > this" patch.
> 
> Thank you.
> 
> > Honestly speaking, I do not understand why Nicolas changed my patch at
> > all.
> > 
> > This patch wastes an extra sed process
> 
> Should we really worry about that in a script like git-am.sh? I mean,
> does it matter in a day to day work?

Oh I've forgotten, yes we need it: sed -e '/^$/q' leaves this matching
line to the output. An extra CRLF makes 'grep -v' fail.

> >                                         introduces [[:blank::]] where
> > space and tab inside [] is perfectly adequate, and we know the latter is
> > understood by everybody's sed.
> 
> But is harder to read in editors.
> 
> > The worst part is that this check was moved before the most common case of
> > mbox file for which none of the overhead for this this extra processing is
> > necessary.
> 
> Well, I did this move just because of the logical structure of the code.
> That said, you're right about the overhead.

-- 
Nicolas Sebrecht