John Peacock wrote:
> Finally, the addition of a new feature: Properties as Keywords. Permits
> user defined keywords, allowing properties to be embedded in source code
> files. While not as useful as it might be with inherited properties, it
> is still of some use, if only as a proof of concept for the keywords as
> hash rewrite. I'm not sure that this is a 1.1.0 candidate.
I'm not sure I like the idea of any and all properties all of a sudden
becoming things that can be expanded as keywords. It seems like it will
limit our ability to add new keywords in the future, since it will break
things for people who already use whatever name we decided on as a
property for some other use.
Perhaps the user should have to specify explicitly somewhere (in a local
config file now, server side config file eventually) what properties are
used as keywords? This would also allow the possibility of specifying
alternate names for existing keywords (i.e. as the *BSD's use $FreeBSD$
or $NetBSD$ as alternative spellings for $Id$).
Also some minor comments:
> Patch attached and log entry inline.
>
> John
>
> ==================================================================
> Custom user-defined keywords using properties
>
> * subversion/libsvn_subr/subst.c
> (svn_subst_build_keywords2): Add stanza to use properties as keywords
>
> * subversion/tests/clients/cmdline/prop_tests.py
> (keyword_props): new tests to exercise properties as keywords feature
>
>
> ------------------------------------------------------------------------
>
> Index: subversion/libsvn_subr/subst.c
> ==================================================================
> --- subversion/libsvn_subr/subst.c (revision 8737)
> +++ subversion/libsvn_subr/subst.c (local)
> @@ -429,6 +429,64 @@
> * placed here to check and see if one of the repository-defined
> * keywords matches
> */
> + else /* a keyword that doesn't match (may be a property) */
> + {
> + if (props)
> + {
> + svn_string_t *prop_val =
> + apr_hash_get(props, keyword, strlen(keyword));
> +
> + /* do no harm if they didn't set that property */
> + if (prop_val)
> + {
> + char *found_nl;
> + svn_error_t *warn = NULL;
> + svn_stringbuf_t *prop_buf =
> + svn_stringbuf_create_from_string (prop_val, pool);
> + apr_size_t last_char = prop_buf->len;
> +
> + /* make sure the property is only a single line */
> + if ((found_nl = strchr (prop_buf->data, '\n')) != NULL)
> + {
> + warn = svn_error_createf (SVN_ERR_BAD_PROP_KIND, NULL,
> + "Property used as keyword "
> + "contains newline: '%s'\n",
> + keyword);
> +
> + /* shorten the string to the newline character */
> + last_char = (apr_size_t)(found_nl - prop_buf->data);
> + }
> +
> + /* cannot have more than SVN_KEYWORD_MAX_LEN characters
> + * in a keyword value
> + */
> + if ( last_char > SVN_KEYWORD_MAX_LEN )
> + {
> + warn = svn_error_createf (SVN_ERR_BAD_PROP_KIND, NULL,
> + "Property used as keyword "
> + "contains too many characters:"
> + "'%s'\n", keyword);
> +
> + last_char = SVN_KEYWORD_MAX_LEN;
> + }
> +
> + /* emit a warning if one has been created */
> + if (warn)
> + {
> + svn_handle_warning (stderr, warn);
> + }
You can't just spew stuff to stderr inside library code. If we need a
way to handle a warning, you should add a callback function/baton pair,
so it can be called when warnings occur.
> + /* chop any extra characters off */
> + if (last_char < prop_buf->len)
> + svn_stringbuf_chop (prop_buf,
> + (prop_buf->len - last_char));
> +
> + apr_hash_set (kw, keyword,
> + APR_HASH_KEY_STRING,
> + svn_string_create_from_buf(prop_buf,pool));
> + }
> + }
> + }
I'm also not especially thrilled with the 'bail if it has a newline' and
'cut it off at some arbitrary size' stuff... I suppose we can't avoid
the newline thing, but the arbitrary size really bothers me...
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Apr 23 20:49:18 2004