On Tue, 2004-06-15 at 23:12, John Peacock wrote:
> > It seems like your patch does more than this; it adds an (undocumented)
> > props parameter to svn_subst_build_keywords2,
>
> See this:
It needs to be in the doc string for svn_subst_build_keywords2, not just
"somewhere."
> > and it introduces a
> > keyword_printf framework replacing the hardcoded keyword constructions
> > in the old svn_subst_build_keywords.
>
> Well, duh, that was the point of the exercise after all. ;)
I thought the point of the exercise was to replace the keyword structure
with a hash.
> The printf() style formatting of keywords was to allow custom keywords to built
> up out of the primitives (see the definition of SVN_KEYWORD_ID_FORMAT in
> particular).
I don't understand the vision here. You seemed to be moving in the
direction of simply allowing the user to put "propname" into
svn:keywords, and have "$propname$" expand to "$propname: propval$". I
don't see where the formatting comes in.
> > It appears that the proper API of svn_subst_build_keywords2 is going to
> > depend on the eventual nature of our custom keyword support. I don't
> > remember seeing, nor can I think up, any practical use cases for the
> > idea of simply allowing properties to be used as keywords
>
> Then you haven't been paying that close attention to the user list! <ouch>
I've observed that developers will commonly attempt to co-opt user
requests into feature ideas that don't fit them (particularly in the
realm of revision labels, but in other areas as well). So simply
telling me to follow the user list more closely isn't enough.
Fortunately, you provided specific examples.
> My personal use case for properties as keywords is a Version property, that will
> be substituted into $Version$ in the source (Perl in my case). Since, IMNSHO,
> the $Revision$ keyword is almost completely useless on a human-readable level
> for multi project repositories, this is my way of propagating a version number
> into my source file.
> I have hope that there will eventually be inheritable properties
The feature seems only barely useful without inheritable properties, in
fact ("svn propset" is more scriptable than a file edit, but that's
it). Inheritable properties are a nifty idea, but they're a huge can of
worms, so I'm reluctant to introduce features which aren't useful in
their absence.
> I've already got a third patch (which goes along with the other two in the
> issue), which adds generic property as keyword (with tests even!), but since it
> has to limit keyword expansion to a single line (and SVN_KEYWORD_MAX_LEN
> characters) I am having a conceptual problem figuring out how to throw a warning
> from inside lib_subst...
There are many contexts in which a warning would be useless anyway (e.g.
if we ever decide to do keyword substitution for an HTTP GET request).
If we do this, I think the right answer is to (a) eliminate the
SVN_KEYWORD_MAX_LEN restriction, and (b) just silently truncate at the
first newline.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jun 16 05:37:31 2004