Re: [PATCHv3] git-p4: add initial support for RCS keywords

Eric Scouten [off-list ref] writes:

quoted

quoted

r'\$(Id|Header|Author|Date|DateTime|Change|File|Revision)[^$]*\$'

Still no ":"?  Won't that match too much?

quoted

Fix the colon thing at least, then happy to add my Acked-By.

No, that would be an incorrect change. The colon is added by P4 when
it expands the keyword pattern, but it is *not* part of the pattern
required by P4 to trigger a keyword expansion.

http://kb.perforce.com/article/54/using-rcs-keywords

I have this suspicion that both Pete and your last sentence is correct,
but the regexp in the patch and your "would be an incorrect change" are
wrong.

I am not a P4 expert, but I would be very surprised if P4 expands "$Ida$"
as if it is "$Id$" or "$Id: old expansion$", which the regexp would match.

Wouldn't it be more like this?

	\$			# begins with a dollar, followed by...
        ( Id | Header | ... )	# one of these keywords, followed by ...
        ( :[^$]+ )?		# possibly an old expansion, followed by
	\$			# another dollar sign

On Tue, Feb 21, 2012 at 09:25, Junio C Hamano [off-list ref] wrote:

Eric Scouten [off-list ref] writes:

quoted

quoted

quoted

r'\$(Id|Header|Author|Date|DateTime|Change|File|Revision)[^$]*\$'

Still no ":"?  Won't that match too much?

quoted

Fix the colon thing at least, then happy to add my Acked-By.

No, that would be an incorrect change. The colon is added by P4 when
it expands the keyword pattern, but it is *not* part of the pattern
required by P4 to trigger a keyword expansion.

http://kb.perforce.com/article/54/using-rcs-keywords

I have this suspicion that both Pete and your last sentence is correct,
but the regexp in the patch and your "would be an incorrect change" are
wrong.

I am not a P4 expert, but I would be very surprised if P4 expands "$Ida$"
as if it is "$Id$" or "$Id: old expansion$", which the regexp would match.

Wouldn't it be more like this?

       \$                      # begins with a dollar, followed by...
       ( Id | Header | ... )   # one of these keywords, followed by ...
       ( :[^$]+ )?             # possibly an old expansion, followed by
       \$                      # another dollar sign

Good catch. Yes, you're probably right.

-- 
Eric Scouten :: software developer, photographer :: Poulsbo, WA (near Seattle)
http://ericscouten.com :: click for Flickr, Facebook, Twitter, LinkedIn links