Re: CVS Keyword expansion patch

[Mark D. Baushke]

Scott James Remnant <sjr@demon.net> writes:

> On Wed, 2003-03-12 at 15:12, Mark D. Baushke wrote:
> 
> I think you misunderstand the intent of the patch, it only looks at the
> colon immediately after the keyword.

Yes, I did misunderstand the patch (I guess I must needed a bit more
caffeine or something to kickstart my brain this morning... :-)

For the case at hand, I am under the impression that PERL allows you to
use ${Header::fields}{$foo} as an alternative method of specifying
$Header::fields{$foo} in most perl 5.001 and beyond releases.

I suspect it be safer to code defensively rather than worry which
version of cvs a user might be using with that code.

Back your your proposed patch...

I think I understand the intent of the patch and I have no problems with
the general goal of making rcs keyword expansion do what users mean it
to do. In this case, that appears to mean restricting the patterns that
are considered for keyword substitution from $keyword$ and $keyword:.*$
to something other criteria like '$keyword$' '$keyword:$' and 
'$keyword: .*$' right?

So a more general patch might be that the character after a colon must
be either whitespace or a $ in order to allow the keyword expansion to
take place.

I would be interested to learn what Paul Eggert thinks of your patch and
the possibility of doing something similar to RCS as I would really
rather that whatever mechanism RCS uses is the same as what CVS uses. It
makes it easier to document and keep things straight.

Note: I have only recently been given the ability to commit to the
repository, so I'd still like to have Larry or Derek bless all features
that cause a behavior change like this one before they get committed.

If you want to get them to bless your patch, you will want to provide a
sanity.sh test case for the new feature as well as the changes needed to
the documentation files.

You should problably open an enhancement request on the cvshome.org web
site for this feature.

        Thanks,
        -- Mark

> From: Scott James Remnant <sjr@demon.net>
> To: bug-cvs@gnu.org
> Date: 12 Mar 2003 11:44:38 +0000
> Subject: CVS Keyword expansion patch
> 
> 
> A simple patch for you to consider,
> 
> It's possible to trip over Keyword expansion when using Perl because
> code like " $Header::fields{$value} " ends up being quite a common
> occurrence.
> 
> CVS would see the "$Header:.....$" in the above and you end up with
> bogus code.  Simply setting -kb isn't the right thing, because you
> generally DO want keyword expansion in the comments at the top, amongst
> other places.
> 
> I therefore submit this patch, basically it prevents keyword expansion
> if the keyword ends in "::"
> 
> ----8<--------8<--------8<--------8<--------8<--------8<--------8<----
> diff -urN cvs-1.11.5.orig/src/rcs.c cvs-1.11.5/src/rcs.c
> --- cvs-1.11.5.orig/src/rcs.c Thu Jan 16 15:10:55 2003
> +++ cvs-1.11.5/src/rcs.c      Wed Mar 12 11:39:08 2003
> @@ -3689,6 +3689,11 @@
>             keyword after all.  */
>       if (*s == ':')
>       {
> +         /* If the keyword ends with '::', then this wasn't a keyword
> +            after all, but a Perl variable!  */
> +         if (*(s + 1) == ':')
> +             continue;
> +
>           for (; s < send; s++)
>               if (*s == '$' || *s == '\n')
>                   break;
> ---->8-------->8-------->8-------->8-------->8-------->8-------->8----
> 
> Scott
> -- 
> sjr  software and systems engineer
>      demon internet, thus plc