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

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

 



The 16/07/09, Junio C Hamano wrote:
> Stephen Boyd <bebarino@xxxxxxxxx> 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?

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