On Thu, 2008-07-03 at 02:59 -0700, Peter Wemm wrote:
> On Wed, Jul 2, 2008 at 1:39 PM, Julian Foad <julianfoad_at_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_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-04 16:34:04 CEST