[Top][All Lists]

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

Re: [PATCH] Add --lt-* options to shell wrapper

From: Ralf Wildenhues
Subject: Re: [PATCH] Add --lt-* options to shell wrapper
Date: Wed, 17 Feb 2010 07:53:32 +0100
User-agent: Mutt/1.5.20 (2009-10-28)

* Charles Wilson wrote on Sun, Feb 14, 2010 at 03:36:57PM CET:
> Charles Wilson wrote:
> > Charles Wilson wrote:
> >> Charles Wilson wrote:
> >>> This one, I think is OK for pre-2.2.8 -- what do you guys think?
> >>> OK to push?
> >> In response to review comments over here:
> >> "Re: [PATCH] Enable runtime cwrapper debugging; add tests"
> >>
> >>
> >> I've created a followon patch to this one which allows the cwrapper
> >> tests to pass on platforms which don't use the cwrapper but instaed use
> >> the shell wrapper (e.g. linux).
> > 
> > ping...
> > 
> ping again.

The option parsing in the original patch is overkill -- no need to
re-quote things if all you're going to do is remove a couple of entries
from "$@", that can be done with
  set x "$@"

type handling.  The reference to _AC_INIT_PREPARE is not needed.

Did you consider that the program we're wrapping might have argument
structure like
  --some-option some-arbitrary-argument-to-this-option

and that the arbitrary argument might reasonably start with --lt-?
Just sayin.

The followon patch adds even more bloat for $LINENO which I don't
understand what you're guarding against, and who this is trying to

I know you deserve a better review, but I've been AFK and the 72 hours
are almost over.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]