kevin@tigris.org writes:
> Log:
> Migrate svn_client_propget to use an apr_hash_t instead of and apr_table_t,
> as suggested by kfogel and gstein. Also improve the docstring, also at the
> suggestion of, and with the aid of kfogel.
>
> * svn_client.h:svn_client_propget - New docstring, and function now
> returns properties in an apr_hash_t.
>
> * libsvn_client/prop_commands.c:recursive_propget,svn_client_propget
> - Adjust to use an apr_hash_t of char *'s and svn_string_t's instead
> of an apr_table_t.
>
> * clients/cmdline/propget-cmd.c:print_prop - Adjust parameters to match
> those returned by svn_client_propget.
>
> * clients/cmdline/propget-cmd.c:svn_cl__propget - Adjust to use new
> parameters to svn_client_propget.
Kevin, one more minor suggestion (and it's my fault for not noticing
and mentioning earlier):
Can you use a similar log entry format to the other developers? The
main difference is to group the symbol names in parens, this makes it
much easier to vgrep. With them all bunched up together, right next
to the file name, with no spaces and no parens, it's actually a lot
harder to browse the entry. Automatic parsing is actually less of a
concern, imho, although keeping to the standard format will of course
make that easier too.
Virtually anyone else's log entry can be used as an example, if you
need, as people have been pretty consistent in using this format.
Again, no biggie -- the change itself looks excellent, and the "###"
comments in propget-cmd.c were a good idea.
Thanks,
-K
> Modified: trunk/subversion/include/svn_client.h
> ==============================================================================
> --- OLD/trunk/subversion/include/svn_client.h Tue Nov 6 17:21:53 2001
> +++ NEW/trunk/subversion/include/svn_client.h Tue Nov 6 17:21:53 2001
> @@ -455,15 +455,22 @@
> svn_boolean_t recurse,
> apr_pool_t *pool);
>
> -/* Returns an apr_table_t of filenames and property values, in *PROPS,
> - allocated in POOL. If TARGET is a file or RECURSE is false, there will
> - be only a single element, with key TARGET, and value the value of PROPNAME
> - in TARGET. If recurse is true and TARGET is a directory, the *PROPS will
> - contain a list of node names and property values for TARGET and all of
> - its children. The nodenames will be of rooted from the same place as
> - TARGET. */
> +/* Set *PROPS to a hash table whose keys are `char *' paths,
> + prefixed by TARGET, of items in the working copy on which
> + property PROPNAME is set, and whose values are `svn_string_t *'
> + representing the property value for PROPNAME at that path.
> + Allocate *PROPS, its keys, and its values in POOL.
> +
> + Don't store any path, not even TARGET, if it does not have a
> + property named PROPNAME.
> +
> + If TARGET is a file or RECURSE is false, *PROPS will have
> + at most one element.
> +
> + If error, don't touch *PROPS, otherwise *PROPS is a hash table even if
> + empty. */
> svn_error_t *
> -svn_client_propget (apr_table_t **props,
> +svn_client_propget (apr_hash_t **props,
> svn_stringbuf_t *propname,
> svn_stringbuf_t *target,
> svn_boolean_t recurse,
>
> Modified: trunk/subversion/libsvn_client/prop_commands.c
> ==============================================================================
> --- OLD/trunk/subversion/libsvn_client/prop_commands.c Tue Nov 6 17:21:53 2001
> +++ NEW/trunk/subversion/libsvn_client/prop_commands.c Tue Nov 6 17:21:53 2001
> @@ -103,7 +103,7 @@
>
> /* Helper for svn_client_propget. */
> static svn_error_t *
> -recursive_propget (apr_table_t *prop_table,
> +recursive_propget (apr_hash_t *props,
> svn_stringbuf_t *propname,
> svn_stringbuf_t *target,
> apr_pool_t *pool)
> @@ -141,7 +141,7 @@
> {
> if (current_entry->kind == svn_node_dir && current_entry_name)
> {
> - SVN_ERR (recursive_propget (prop_table, propname,
> + SVN_ERR (recursive_propget (props, propname,
> full_entry_path, pool));
> }
> else
> @@ -150,9 +150,8 @@
> SVN_ERR (svn_wc_prop_get (&propval, propname, full_entry_path,
> pool));
> if (propval)
> - apr_table_set (prop_table,
> - full_entry_path->data,
> - propval->data);
> + apr_hash_set (props, full_entry_path->data, APR_HASH_KEY_STRING,
> + svn_string_create_from_buf (propval, pool));
> }
> }
> }
> @@ -160,13 +159,13 @@
> }
>
> svn_error_t *
> -svn_client_propget (apr_table_t **props,
> +svn_client_propget (apr_hash_t **props,
> svn_stringbuf_t *propname,
> svn_stringbuf_t *target,
> svn_boolean_t recurse,
> apr_pool_t *pool)
> {
> - apr_table_t *prop_table = apr_table_make (pool, 5);
> + apr_hash_t *prop_hash = apr_hash_make (pool);
> svn_wc_entry_t *node;
>
> SVN_ERR (svn_wc_entry(&node, target, pool));
> @@ -178,17 +177,18 @@
>
> if (recurse && node->kind == svn_node_dir)
> {
> - SVN_ERR (recursive_propget (prop_table, propname, target, pool));
> + SVN_ERR (recursive_propget (prop_hash, propname, target, pool));
> }
> else
> {
> svn_stringbuf_t *propval;
> SVN_ERR (svn_wc_prop_get (&propval, propname, target, pool));
> if (propval)
> - apr_table_set(prop_table, target->data, propval->data);
> + apr_hash_set(prop_hash, target->data, APR_HASH_KEY_STRING,
> + svn_string_create_from_buf (propval, pool));
> }
>
> - *props = prop_table;
> + *props = prop_hash;
> return SVN_NO_ERROR;
> }
>
>
> Modified: trunk/subversion/clients/cmdline/propget-cmd.c
> ==============================================================================
> --- OLD/trunk/subversion/clients/cmdline/propget-cmd.c Tue Nov 6 17:21:53 2001
> +++ NEW/trunk/subversion/clients/cmdline/propget-cmd.c Tue Nov 6 17:21:53 2001
> @@ -34,10 +34,12 @@
> /*** Code. ***/
>
> static int
> -print_prop(void *propname, const char *filename, const char *propval)
> +print_prop(const svn_string_t *propname,
> + const char *filename,
> + const svn_string_t *propval)
> {
> - const char *pn = propname;
> - printf("%s - %s : %s\n", filename, pn, propval);
> + /* ### This won't handle binary property values properly. */
> + printf("%s - %s : %s\n", filename, propname->data, propval->data);
> return 1;
> }
>
> @@ -65,12 +67,22 @@
> for (i = 0; i < targets->nelts; i++)
> {
> svn_stringbuf_t *target = ((svn_stringbuf_t **) (targets->elts))[i];
> - apr_table_t *props;
> + /* ### Main code should propably be changed to make arguments
> + svn_string_t's instead of svn_stringbuf_t's */
> + svn_string_t pname = { propname->data, propname->len };
> + apr_hash_t *props;
> + apr_hash_index_t *hi;
>
> SVN_ERR (svn_client_propget (&props, propname, target,
> opt_state->recursive, pool));
>
> - apr_table_do(&print_prop,propname->data, props, NULL);
> + for (hi = apr_hash_first(pool, props); hi; apr_hash_next(hi))
> + {
> + const char * filename;
> + const svn_string_t *propval;
> + apr_hash_this(hi, (const void **)&filename, NULL, (void **)&propval);
> + print_prop(&pname, filename, propval);
> + }
> }
>
> return SVN_NO_ERROR;
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:48 2006