You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@newton.ch.collab.net> on 2001/11/07 16:26:09 UTC
Re: svn commit: rev 416 - trunk/subversion/include trunk/subversion/libsvn_client trunk/subversion/clients/cmdline
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
Re: svn commit: rev 416 - trunk/subversion/include trunk/subversion/libsvn_client trunk/subversion/clients/cmdline
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Kevin Pilch-Bisson <ke...@pilch-bisson.net> writes:
> > Kevin, one more minor suggestion (and it's my fault for not noticing
> > and mentioning earlier):
>
> Always welcome, and lets stay away from the issue of fault:)
Sorry, my fault for bringing it up.
:-),
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 416 - trunk/subversion/include trunk/subversion/libsvn_client trunk/subversion/clients/cmdline
Posted by Kevin Pilch-Bisson <ke...@pilch-bisson.net>.
On Wed, Nov 07, 2001 at 10:26:09AM -0600, Karl Fogel wrote:
> Kevin, one more minor suggestion (and it's my fault for not noticing
> and mentioning earlier):
Always welcome, and lets stay away from the issue of fault:)
>
> 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.
>
Will do in the future, sorry.
>
> 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
>
--
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson http://www.pilch-bisson.net
"Historically speaking, the presences of wheels in Unix
has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~