[aur-dev] [PATCH] Fix: FS#13122 - simpler commented line removal

[Loui Chang]

On Tue, Feb 17, 2009 at 10:08:50AM +0800, Gergely Imreh wrote:
> 2009/2/17 Loui Chang <louipc.ist at gmail.com>:
> > On Mon, Feb 16, 2009 at 10:45:33PM +0800, Gergely Imreh wrote:
> >> How about this new version? Handles FS#13122 and FS#13173 well - by
> >> which I mean that it does not remove anything that is not comment. The
> >> # characters intended for bash parameter replacement are kept (so not
> >> breaking the upload), though not handled yet within the replacement
> >> section. That is for another day.
> >
> > I pushed a modded version of your patch, so it's attributed to you.
> > It only neutralises the parameter substitution, though.
> >
> >
> 
> 
> You shouldn't attribute that commit to me, because it has absolutely
> nothing to do with the patch I sent in, it's totally yours... I'm not
> even sure what "neutralises" mean in this situation, yet.... Got to
> figure out the changes now. :)

Hah, well you wrote the patch that it was based on, and you even wrote
the code that it was patching. ;)
By 'neutralises' I mean that it makes the substitution no longer a
substitution, so the rest of the PKGBUILD parsing doesn't trip over it.
I could have been clearer about that.

> As for your previous comment about foo=bar;#none, that could be
> handled with my version with some change, using this regular
> expression replacement:
> $line = preg_replace('/(;*)(^|\s+)#.*/', "$1", $line);
> This version handles things as:
> #none -> (empty line)
> foo=bar;#none -> foo=bar;
> foo=bar #none -> foo=bar
> foo=bar          #none -> foo=bar
> foo=${bar#none} -> foo=${bar#none}
> foo=${bar##none} -> foo=${bar##none}
> foo=${bar #none} -> foo=${bar
> The last one would result we handle in the parameter substitution
> (although wrong), but that's fine since it breaks bash, so it's a
> wrong PKGBUILD.

OK. You're welcome to change what I pushed if you're continuing patching
the parser. I just wanted to push some quick fixes. That infinite loop
bug might have been part of some real problems on the server.
The changes are live, but I forgot to update the version string. Doh.
No big deal though.

> Is there any other case that is not handled?

I'm not really sure to be honest. I'm flying by the seat of my pants on
this really. I don't think anyone's bothered to improve the parsing
until now because there are so many possibilities to cover.
Your work has really helped cover a nice chunk of them.

If we really want to protect against surprises we'd have to read the
manual on bash at least a few times. I'm alright with letting users
discover the inaccuracies of the parser.

Side note: It might be a good idea to split the parsing into its own
script and/or set of functions.