[Top][All Lists]

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

Re: [Fab-user] Bug in multiline append

From: Christian Vest Hansen
Subject: Re: [Fab-user] Bug in multiline append
Date: Fri, 22 May 2009 15:04:18 +0200

Why do you make that check in append in the first place?

"append" sounds like a procedure that does one thing: appending. Not
checking if something is already in the file and then maybe adding it
to the end. That sounds like something that could be called
"ensureContains" or something.

Also, you could consider using fgrep or grep -F to get fixed-string
matching instead of regexp - this is perhaps an extension you could
make to "contains".

On Fri, May 22, 2009 at 1:45 PM, Jeff Forcier <address@hidden> wrote:
>> Ideally I think this is a case where the internals should be messy to
>> leave the external api clean. You do not want regexp in the high level
>> file functions (append, replace, comment, etc.) or do we?
> Well, in some cases we *do*, because of the heavy grep/awk/etc usage
> in these functions -- i.e.comment() and contains() have to take
> regexes because that's *all* they do: search for lines matching a
> regex.
> However, with append(), as you noted, I left things in the unfortunate
> state where the same input string is used as *both* a regex (the
> interal contains() call) *and* a non-regex (the actual append). That
> was a mistake on my part, and wasn't caught since my use of the
> function has so far been limited to strings not containing
> regex-specific characters :)
> This needs to be fixed such that every string argument is clearly
> defined as being used as a regex, or not, and then the question is
> whether we fix append() by implicitly trying to make its single string
> argument regex-safe (hopefully by leveraging some builtin function
> like something in the 're' module, if one exists); by splitting that
> into two arguments; or simply by removing the contains() check
> entirely.
> -Jeff
> _______________________________________________
> Fab-user mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/fab-user

Venlig hilsen / Kind regards,
Christian Vest Hansen.

reply via email to

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