You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Peter Wemm <pe...@wemm.org> on 2008/07/03 09:59:09 UTC
Re: More custom keywords [was: bug: invalid svn:keywords silently ignored]
On Wed, Jul 2, 2008 at 1:39 PM, Julian Foad <ju...@btopenworld.com> wrote:
> On Thu, 2008-06-26 at 11:37 +0200, Stefan Sperling wrote:
>> On Thu, Jun 26, 2008 at 01:48:23AM -0700, Peter Wemm wrote:
>> > The custom keywords change was done with an eye towards submitting it.
>> > I was inspired by the patch in the issue database, but there were a
>> > number of serious problems with it. And I wanted something truely
>> > flexible.
>> >
>> > FWIW, the version we're running right now is here:
>> > http://people.freebsd.org/~peter/svn_keyword.diff
>> > Unlike some of our other hacks, this one is non invasive. It doesn't
>> > affect svn's operation unless the features are activated via
>> > properties.
>>
>> The patch says it was for issue #860, but you meant #890, right?
>>
>> "Implementation of Custom keywords"
>> http://subversion.tigris.org/issues/show_bug.cgi?id=890
>>
>> Would you mind if I added the URL to the patch to issue #890?
>> Your patch is certainly worth tracking, even if you don't think
>> it's ready for submission yet.
>
> I'm sure Peter won't mind it being linked from issue #890, so I've done
> that.
Thanks! Firstly, I must apologize - I've been exceedingly busy over
the last few weeks. I'm still fighting the remaining fires with
converting freebsd.org from cvs to svn, as well as dealing with an
emergency involving lawyers. I owe cleaned up patches and bug reports
to a bunch of people. The quality of the patches have degenerated
into downright evil hacks as it came closer to crunch time. :)
> 4. Add some aliases for format strings. Where the format string (after
> an equals sign in svn:keywords) is "%H" or "%I", interpret it as:
>
> * "%H" => "%P %r %d %a" (like CVS's $CVSHeader$)
> * "%I" => "%b %r %d %a" (like $Id$)
>
> To specify these expansions in full in svn:keywords you'd have to write
> them with the spaces replaced by %_, like "%P%_%r%_%d%_%a".
...
> Peter says in the patch file:
>
>> # http://people.freebsd.org/~peter/svn_keyword.diff
...
>> # and as aliases in the keyword builder:
>> # %H - alias for $CVSHeader$ expansion. (%P %r %d %a)
>> # %I - alias for $Id$ expansion. (%b %r %d %a)
>> # the keyword builder aliases are because the svn:keyword splitter
>> # uses spaces.
>
> I don't quite follow the reasoning. As you have added "%_" to mean a
> space, these two aliases "%H" and "%I" don't seem necessary. Are they
> just conveniences to avoid having to write something like "MyKeyword=%P%
> _%r%_%d%_%a" ?
I added and removed and re-added the %_ for space thing. I'm not
happy with it. Typing all those %'s gets old fast and is pretty harsh
on the eyes.
>> # freebsd.org uses this patch, relative to 1.5, and sets
>> # svn:keywords to "FreeBSD=%H'. This expands thus:
>> # $FreeBSD: stable/7/bin/ls/ls.c 157160 2006-03-24 17:09:03Z jhb $
>> # $FreeBSD: head/bin/ls/ls.c 177989 2008-04-04 03:57:46Z grog $
>> # This works the same way as $CVSHeader$ in gnu-cvs.
>
> I have lightly reviewed this patch and don't see any major problems. I
> haven't looked at the edge cases and exception handling (like what if
> there is a dollar sign in the keyword expansion value, and so on). I did
> notice that the aliases "%H" and "%I" are restrictive in that you can't
> use them within a longer expression such as "%R%_%I".
Yes. That is something else I've worried about. One thing I would
have liked to have done is generalized it a bit more. cvs-1.12 picked
up the hack functionality that we added to cvs many years ago. In its
options file you can now specify things like:
./src/mkmodules.c: "#LocalKeyword=MYCVS=CVSHeader\n",
./src/mkmodules.c: "#KeywordExpand=eCVSHeader\n",
cvs now allows you to rename or alias keywords in terms of other keywords.
I'd have liked to have done something a bit more general. I thought
many times about adding a longer form of $Id$ ala $CVSHeader$. I
thought of having $Header$ expand like $Id$ except with the
repos-relative path instead of just the basename. Then remove the
reference restrictions in the alias/renames so that you could have
svn:keywords 'FreeBSD=Header' and allow %R%_%I if you wanted. I was
trying to keep the patch footprint to a minimum though.
If you'll indulge me with a little more time to fight the remaining
fires and let the dust settle at freebsd.org, I'd like to take another
shot at making this a little less unpleasant to look at. I think I
can do better.
Cheers,
-Peter
--
Peter Wemm - peter@wemm.org; peter@FreeBSD.org; peter@yahoo-inc.com; KI6FJV
"All of this is for nothing if we don't go to the stars" - JMS/B5
"If Java had true garbage collection, most programs would delete
themselves upon execution." -- Robert Sewell
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: More custom keywords [was: bug: invalid svn:keywords silently
ignored]
Posted by Julian Foad <ju...@btopenworld.com>.
On Thu, 2008-07-03 at 02:59 -0700, Peter Wemm wrote:
> On Wed, Jul 2, 2008 at 1:39 PM, Julian Foad <ju...@btopenworld.com> wrote:
> > On Thu, 2008-06-26 at 11:37 +0200, Stefan Sperling wrote:
> >> On Thu, Jun 26, 2008 at 01:48:23AM -0700, Peter Wemm wrote:
> >> > The custom keywords change was done with an eye towards submitting it.
> >> > I was inspired by the patch in the issue database, but there were a
> >> > number of serious problems with it. And I wanted something truely
> >> > flexible.
> >> >
> >> > FWIW, the version we're running right now is here:
> >> > http://people.freebsd.org/~peter/svn_keyword.diff
> >> > Unlike some of our other hacks, this one is non invasive. It doesn't
> >> > affect svn's operation unless the features are activated via
> >> > properties.
> >>
> >> The patch says it was for issue #860, but you meant #890, right?
> >>
> >> "Implementation of Custom keywords"
> >> http://subversion.tigris.org/issues/show_bug.cgi?id=890
> >>
> >> Would you mind if I added the URL to the patch to issue #890?
> >> Your patch is certainly worth tracking, even if you don't think
> >> it's ready for submission yet.
> >
> > I'm sure Peter won't mind it being linked from issue #890, so I've done
> > that.
>
> Thanks! Firstly, I must apologize - I've been exceedingly busy over
> the last few weeks. I'm still fighting the remaining fires with
> converting freebsd.org from cvs to svn, as well as dealing with an
> emergency involving lawyers. I owe cleaned up patches and bug reports
> to a bunch of people. The quality of the patches have degenerated
> into downright evil hacks as it came closer to crunch time. :)
>
>
> > 4. Add some aliases for format strings. Where the format string (after
> > an equals sign in svn:keywords) is "%H" or "%I", interpret it as:
> >
> > * "%H" => "%P %r %d %a" (like CVS's $CVSHeader$)
> > * "%I" => "%b %r %d %a" (like $Id$)
> >
> > To specify these expansions in full in svn:keywords you'd have to write
> > them with the spaces replaced by %_, like "%P%_%r%_%d%_%a".
> ...
> > Peter says in the patch file:
> >
> >> # http://people.freebsd.org/~peter/svn_keyword.diff
> ...
> >> # and as aliases in the keyword builder:
> >> # %H - alias for $CVSHeader$ expansion. (%P %r %d %a)
> >> # %I - alias for $Id$ expansion. (%b %r %d %a)
> >> # the keyword builder aliases are because the svn:keyword splitter
> >> # uses spaces.
> >
> > I don't quite follow the reasoning. As you have added "%_" to mean a
> > space, these two aliases "%H" and "%I" don't seem necessary. Are they
> > just conveniences to avoid having to write something like "MyKeyword=%P%
> > _%r%_%d%_%a" ?
>
> I added and removed and re-added the %_ for space thing. I'm not
> happy with it. Typing all those %'s gets old fast and is pretty harsh
> on the eyes.
>
> >> # freebsd.org uses this patch, relative to 1.5, and sets
> >> # svn:keywords to "FreeBSD=%H'. This expands thus:
> >> # $FreeBSD: stable/7/bin/ls/ls.c 157160 2006-03-24 17:09:03Z jhb $
> >> # $FreeBSD: head/bin/ls/ls.c 177989 2008-04-04 03:57:46Z grog $
> >> # This works the same way as $CVSHeader$ in gnu-cvs.
> >
> > I have lightly reviewed this patch and don't see any major problems. I
> > haven't looked at the edge cases and exception handling (like what if
> > there is a dollar sign in the keyword expansion value, and so on). I did
> > notice that the aliases "%H" and "%I" are restrictive in that you can't
> > use them within a longer expression such as "%R%_%I".
>
> Yes. That is something else I've worried about. One thing I would
> have liked to have done is generalized it a bit more. cvs-1.12 picked
> up the hack functionality that we added to cvs many years ago. In its
> options file you can now specify things like:
> ./src/mkmodules.c: "#LocalKeyword=MYCVS=CVSHeader\n",
> ./src/mkmodules.c: "#KeywordExpand=eCVSHeader\n",
> cvs now allows you to rename or alias keywords in terms of other keywords.
>
> I'd have liked to have done something a bit more general. I thought
> many times about adding a longer form of $Id$ ala $CVSHeader$. I
> thought of having $Header$ expand like $Id$ except with the
> repos-relative path instead of just the basename. Then remove the
> reference restrictions in the alias/renames so that you could have
> svn:keywords 'FreeBSD=Header' and allow %R%_%I if you wanted. I was
> trying to keep the patch footprint to a minimum though.
>
> If you'll indulge me with a little more time to fight the remaining
> fires and let the dust settle at freebsd.org, I'd like to take another
> shot at making this a little less unpleasant to look at. I think I
> can do better.
Of course you're welcome to come back to this after a while if you like.
I'll leave it alone, then, until we hear from you again.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org