On 7/23/05 1:57 AM, "Julian Foad" <julianfoad@btopenworld.com> sneezed:
> Indentation of your function argument lists and continuation lines should be
> such that all arguments start in the same column, and continuation lines start
> at the column where the expression started at the corresponding level of
> bracket nesting: e.g.:
>
> foo (a,
> b);
>
> If the open-parenthesis was much too far to the right, move it down and indent
> by two spaces:
>
> foo_bar
> (a,
> b);
>
This really made me laugh. I've been subscribed to the dev-list for ~= 2
months now and have seen so many patches scrutinized for coding style, and
honestly thought, "What bullshit!." You were all a bunch of little monkies
scrambling around to preserve the most god-awful, un-readable, code
formatting. With those last two paragraphs it finally reached
me...mono-spaced fonts! The formatting was so confusing to me that I was
actually copying and pasting brackets, unrelated code/function, and more
just to get their indentation. Anyway, I've switched my editor's font, so
hopefully formatting is less of an issue this time.
> It is easier for me to look at your patches when you attach them with a MIME
> type like text/plain, like you did before, rather than
> application.octet-stream
> which it is this time. That's just a request for next time.
I really have no idea what Entourage is doing, so I hope this is better (not
that I'm doing anything different from the last two times).
> I'm not sure that the ability to choosing absolute or relative paths is a
> good idea. Just do the same as we do in the other XML outputs.
Relative?
> Please could you supply an XML DTD file to go with this, modeled on
> subversion/clients/cmdline/dtd/*.dtd?
Once the tags/patch have been finalized/approved I will...more below.
> Some final unfinished thoughts:
>
> These function typedefs and "void *baton"s ... I'm not sure if they are being
> used in an appropriate manner.
The manner is for code reuse and bloat avoidance.
> I don't like the bit where you cast a Boolean value into a "void *" pointer
value.
When I first typed it I too thought it was weird, but in the end it's no
weirder than casting false/null to a void*. Any void* is potentially
harmful if misinterpreted.
It seems pointless/sub-optimal to me, but I really don't care if:
a.) the boolean's address is passed and deferenced
b.) both function's receive an svn_stringbuf_t* and boolean paramter.
> XML element names should match those in the existing DTDs where possible.
I never noticed subversion/clients/cmdline/dtd/ before but had a look. I
was modeling the xml on Apple's plists, because, well that¹s what I'll need
this for.
In this patch I'm actually parsing the data in the property to infer if its
of seven types rather then using strictly string or data. Its important to
do, otherwise if anyone else starts using this functionality, there's going
to be a bunch of xsl written to accomplish the same thing.
Being said, parsing arbitray values for meaning they might not have is
perhaps weird enough that this patch is rejected. That¹s a question.
> Thanks for taking it this far; I hope you still have time to go through
> another round of patch and review.
Theres more!?
[[[
Add an XML output mode for "svn proplist".
* subversion/subversion/clients/cmdline/cl.h,
subversion/subversion/clients/cmdline/props.c
(svn_cl__print_prop_hash): Remove. It was only being used in
proplist-cmd.c, and needed to be re-factored to print different formats.
* subversion/subversion/clients/cmdline/proplist-cmd.c:
(svn_cl__proplist) Modify. Test for opt_state->xml and acts accordingly.
(iterate_prop_hash): New, moved out from svn_cl__proplist.
(iterate_prop_array): New, based on svn_cl__print_prop_hash.
(print_prop_xml): New. Print a single property in xml.
(print_prop_cl): New. Print a single property to command-line.
(group_props_xml): New. Print a group of properties to xml.
(group_props_cl): New. Print a group of properties to command-line.
(begin_xml_proplist): New. Start a property list xml stringbuf.
(end_xml_proplist): New. End a property list xml stringbuf.
* subversion/subversion/clients/cmdline/main.c:
(svn_cl__cmd_table): Modify. Added svn_cl__xml_opt to proplist command.
]]]
Index: cmdline/props.c
===================================================================
--- cmdline/props.c (revision 15395)
+++ cmdline/props.c (working copy)
@@ -42,41 +42,3 @@
_("Must specify revision explicitly when operating on a "
"revision property"));
}
-
-
-svn_error_t *
-svn_cl__print_prop_hash (apr_hash_t *prop_hash,
- svn_boolean_t names_only,
- apr_pool_t *pool)
-{
- apr_hash_index_t *hi;
-
- for (hi = apr_hash_first (pool, prop_hash); hi; hi = apr_hash_next (hi))
- {
- const void *key;
- void *val;
- const char *pname;
- svn_string_t *propval;
- const char *pname_stdout;
-
- apr_hash_this (hi, &key, NULL, &val);
- pname = key;
- propval = val;
-
- if (svn_prop_needs_translation (pname))
- SVN_ERR (svn_subst_detranslate_string (&propval, propval,
- TRUE, pool));
-
- SVN_ERR (svn_cmdline_cstring_from_utf8 (&pname_stdout, pname, pool));
-
- /* ### We leave these printfs for now, since if propval wasn't translated
- * above, we don't know anything about its encoding. In fact, it
- * might be binary data... */
- if (names_only)
- printf (" %s\n", pname_stdout);
- else
- printf (" %s : %s\n", pname_stdout, propval->data);
- }
-
- return SVN_NO_ERROR;
-}
Index: cmdline/cl.h
===================================================================
--- cmdline/cl.h (revision 15395)
+++ cmdline/cl.h (working copy)
@@ -249,7 +249,6 @@
svn_boolean_t repos_locks,
apr_pool_t *pool);
-
/* Print STATUS for PATH in XML to stdout. Use POOL for temporary
allocations. */
svn_error_t *
@@ -257,19 +256,6 @@
svn_wc_status2_t *status,
apr_pool_t *pool);
-
-/* Print a hash that maps property names (char *) to property values
- (svn_string_t *). The names are assumed to be in UTF-8 format;
- the values are either in UTF-8 (the special Subversion props) or
- plain binary values.
-
- If NAMES_ONLY is true, print just names, else print names and
- values. */
-svn_error_t *
-svn_cl__print_prop_hash (apr_hash_t *prop_hash,
- svn_boolean_t names_only,
- apr_pool_t *pool);
-
/* Return a SVN_ERR_CL_ARG_PARSING_ERROR error, with a message stating
that one must give an explicit revision when operating on a
revision property. */
Index: cmdline/proplist-cmd.c
===================================================================
--- cmdline/proplist-cmd.c (revision 15395)
+++ cmdline/proplist-cmd.c (working copy)
@@ -26,7 +26,11 @@
#include "svn_pools.h"
#include "svn_client.h"
#include "svn_error.h"
+#include "svn_subst.h"
#include "svn_path.h"
+#include "svn_xml.h"
+#include "svn_time.h"
+#include "svn_base64.h"
#include "cl.h"
#include "svn_private_config.h"
@@ -34,6 +38,308 @@
/*** Code. ***/
+/* A function that operates on a single property. The property name is in
+ PNAME, it's value in PROPVAL. A function of this type should be passed to
+ iterate_prop_hash. */
+typedef svn_error_t* (*prop_iter_function_t) (const char* pname,
+ const svn_string_t *propval,
+ void *refcon,
+ apr_pool_t *pool);
+
+/* A function that will arbitrarily group a set of properties if we're
+ iterating properties for more than one file/directory. The properties
+ are contained in PROP_HASH, and are the properties for the item in PATH.
+ PROP_HASH should probably be passed to iterate_prop_hash along with a
+ prop_iter_function_t. */
+typedef svn_error_t* (*prop_group_function_t) (apr_hash_t *prop_hash,
+ const char *path,
+ void *baton,
+ apr_pool_t *pool);
+
+/* Tell iterate_prop_array how to compute the path it will send for printing.
+ The first two values also double as a boolean for whether the path is url.
+*/
+typedef enum {
+ print_path_local,
+ print_path_url,
+ print_path_absolute,
+} print_path_style_t;
+
+/* Iterate a hash that maps property names (char *) to property values
+ (svn_string_t *). The names are assumed to be in UTF-8 format;
+ the values are either in UTF-8 (the special Subversion props) or
+ plain binary values.
+
+ Pass the property name and a (possibly de-translated) value
+ to the print function in PRINT_FUNC. */
+static svn_error_t *
+iterate_prop_hash (apr_hash_t *prop_hash,
+ prop_iter_function_t print_func,
+ void *baton,
+ apr_pool_t *pool)
+{
+ apr_hash_index_t *hi;
+
+ for (hi = apr_hash_first (pool, prop_hash); hi; hi = apr_hash_next (hi))
+ {
+ const char *pname;
+ svn_string_t *propval;
+
+ apr_hash_this (hi, (const void **)&pname, NULL, (void**)&propval);
+
+ if (svn_prop_needs_translation (pname))
+ SVN_ERR (svn_subst_detranslate_string (&propval, propval,
+ TRUE, pool));
+
+ SVN_ERR (print_func (pname, propval, baton, pool));
+ }
+
+ return SVN_NO_ERROR;
+}
+
+/* Iterate an array of props, where each element is a hash that maps
+ property names (char *) to property values (svn_string_t *).
+
+ Pass one hash and the current property's path to the print-function given
+ in GROUP_FUNC. The path is computed as either local, url, or absolute
+ depending on PATH_STYLE. */
+static svn_error_t*
+iterate_prop_array (apr_array_header_t *props,
+ print_path_style_t path_style,
+ prop_group_function_t group_func,
+ void *baton,
+ apr_pool_t *pool)
+{
+ int j;
+ for (j = 0; j < props->nelts; ++j)
+ {
+ svn_client_proplist_item_t *item
+ = ((svn_client_proplist_item_t **)props->elts)[j];
+ const char *name_local;
+
+ switch (path_style)
+ {
+ case print_path_absolute:
+ SVN_ERR (svn_path_get_absolute (&name_local,
+ item->node_name->data, pool));
+ break;
+
+ case print_path_local:
+ name_local = svn_path_local_style (item->node_name->data,
+ pool);
+ break;
+
+ case print_path_url:
+ default:
+ name_local = item->node_name->data;
+ break;
+ }
+
+ SVN_ERR (group_func (item->prop_hash, name_local, baton, pool));
+ }
+ return SVN_NO_ERROR;
+}
+
+/* Print a property in xml, appending the svn_stringbuf_t
+ passed in baton. The property will be catagorized as one of 7 types:
+ <data/> base-64 encoded binary data.
+ <date/> an ISO-8601 date
+ <false/> a boolean value of 0
+ <integer/> an integer number
+ <real/> a real/floating-point number
+ <string/> a string
+ <true/> a boolean value of 1
+
+ PNAME is the property name, PROPVAL it's value.
+ BATON is an svn_stringbuf_t.
+
+ This function is an prop_iter_function_t*/
+static svn_error_t *
+print_prop_xml (const char *pname,
+ const svn_string_t *propval,
+ void *baton,
+ apr_pool_t *pool)
+{
+ svn_stringbuf_t *esc_name = NULL;
+ svn_stringbuf_t *sb = (svn_stringbuf_t*) baton;
+
+ svn_xml_escape_attr_cstring (&esc_name, pname, pool);
+ svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata, "entry",
+ "name", esc_name->data, NULL);
+
+ if (!svn_xml_is_xml_safe (propval->data, propval->len))
+ {
+ svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata, "data",
+ NULL);
+ const svn_string_t *bin_prop = svn_base64_encode_string (propval,
+ pool);
+ svn_xml_escape_cdata_string (&sb, bin_prop, pool);
+ svn_xml_make_close_tag (&sb, pool, "data");
+ }
+ else
+ {
+ long double number;
+ char c;
+ int rslt;
+
+ /* Scan for a number and an extra char, if the char matches, then
+ it's probably string (i.e "90 tomatos added") */
+ rslt = sscanf (propval->data, "%Lf%c", &number, &c);
+ if (rslt==1)
+ {
+ const char *number_type;
+ long double number2 = lround(number);
+
+ /* If new and old floating point numbers are not the same,
+ it's no integer */
+ number_type = (number!=number2 ? "real" : "integer");
+
+ svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
+ number_type, NULL);
+ svn_xml_escape_cdata_string (&sb, propval, pool);
+ svn_xml_make_close_tag (&sb, pool, number_type);
+ }
+ else
+ {
+ svn_error_t *svn_err;
+ apr_time_t result;
+ svn_boolean_t matched;
+
+ /* Ignore any errors on parsing a date...it might not be a date
+ after-all */
+ svn_err = svn_parse_date (&matched, &result, propval->data,
+ apr_time_now(), pool);
+ if (svn_err)
+ svn_error_clear (svn_err);
+ else if (matched)
+ {
+ const char *iso_time = svn_time_to_cstring (result, pool);
+ svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
+ "date", NULL);
+ svn_xml_escape_cdata_cstring (&sb, iso_time, pool);
+ svn_xml_make_close_tag (&sb, pool, "date");
+
+ return SVN_NO_ERROR;
+ }
+ else if ( strcasecmp (propval->data, "true")==0 )
+ {
+ svn_stringbuf_appendcstr (sb, "<true/>\n");
+ return SVN_NO_ERROR;
+ }
+ else if ( strcasecmp (propval->data, "false")==0 )
+ {
+ svn_stringbuf_appendcstr (sb, "<false/>\n");
+ return SVN_NO_ERROR;
+ }
+
+ svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
+ "string", NULL);
+ svn_xml_escape_cdata_string (&sb, propval, pool);
+ svn_xml_make_close_tag (&sb, pool, "string");
+ }
+ }
+
+ svn_xml_make_close_tag (&sb, pool, "entry");
+
+ return SVN_NO_ERROR;
+}
+
+/* Print the property name and, if NAMES_ONLY is non-null, it's value.
+
+ PNAME is the property name, PROPVAL it's value.
+
+ This function is an prop_iter_function_t*/
+static svn_error_t *
+print_prop_cl (const char *pname,
+ const svn_string_t *propval,
+ void *names_only,
+ apr_pool_t *pool)
+{
+ const char *pname_stdout;
+ SVN_ERR (svn_cmdline_cstring_from_utf8 (&pname_stdout, pname, pool));
+ /* ### We leave these printfs for now, since if propval wasn't translated
+ * above, we don't know anything about its encoding. In fact, it
+ * might be binary data... */
+ if (names_only)
+ printf (" %s\n", pname_stdout);
+ else
+ printf (" %s : %s\n", pname_stdout, propval->data);
+
+ return SVN_NO_ERROR;
+}
+
+/* Groups the properties given in PROP_HASH within tag-pair.
+ Appends the svn_stringbuf_t passed in BATON.
+ <target path="_path_">
+ call iterate_prop_hash
+ </target>
+
+ This function is a prop_group_function_t. */
+static svn_error_t*
+group_props_xml (apr_hash_t *prop_hash,
+ const char *path,
+ void *baton,
+ apr_pool_t *pool)
+{
+ svn_stringbuf_t *escpath = NULL;
+ svn_stringbuf_t *sb = (svn_stringbuf_t*) baton;
+
+ /* <target path=" _path_ "> */
+ svn_xml_escape_attr_cstring (&escpath, path, pool);
+ svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata, "target",
+ "path", escpath->data, NULL);
+
+ svn_stringbuf_appendcstr (sb, "\n");
+ SVN_ERR (iterate_prop_hash (prop_hash, print_prop_xml, sb, pool));
+
+ /* </target> */
+ svn_xml_make_close_tag (&sb, pool, "target");
+}
+
+/* Prints the filepath/url for the properties in PROP_HASH.
+ Then calls iterate_prop_hash to print the properties themselves.
+
+ This function is a prop_group_function_t. */
+static svn_error_t*
+group_props_cl (apr_hash_t *prop_hash,
+ const char *path,
+ void *baton,
+ apr_pool_t *pool)
+ {
+ SVN_ERR (svn_cmdline_printf (pool, "Properties on '%s':\n", path));
+ SVN_ERR (iterate_prop_hash (prop_hash, print_prop_cl, baton, pool));
+ return SVN_NO_ERROR;
+}
+
+/* Create an svn_stringbuf_t, writes an xml header, and opens the first tag
+ (<propertylist>). If IS_LIST is true, also opens <list> tag. */
+static svn_error_t *
+begin_xml_proplist (svn_stringbuf_t **sb,
+ svn_boolean_t is_list,
+ apr_pool_t *pool)
+{
+ *sb = svn_stringbuf_create ("", pool);
+ svn_xml_make_header (sb, pool);
+ svn_xml_make_open_tag (sb, pool, svn_xml_normal, "propertylist", NULL);
+ if (is_list)
+ svn_xml_make_open_tag (sb, pool, svn_xml_normal, "list", NULL);
+}
+
+/* Closes the xml tags started by begin_xml_proplist, and writes SB to stdout.
+ If IS_LIST is true, first close <list> tag. */
+static svn_error_t *
+end_xml_proplist (svn_stringbuf_t *sb,
+ svn_boolean_t is_list,
+ apr_pool_t *pool)
+{
+ if (is_list)
+ svn_xml_make_close_tag (&sb, pool, "list");
+ svn_xml_make_close_tag (&sb, pool, "propertylist");
+ SVN_ERR (svn_cl__error_checked_fputs (sb->data, stdout));
+ return SVN_NO_ERROR;
+}
+
+
/* This implements the `svn_opt_subcommand_t' interface. */
svn_error_t *
svn_cl__proplist (apr_getopt_t *os,
@@ -43,6 +349,7 @@
svn_cl__opt_state_t *opt_state = ((svn_cl__cmd_baton_t *) baton)->opt_state;
svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
apr_array_header_t *targets;
+ svn_stringbuf_t *sb;
int i;
/* Suck up all remaining args in the target array. */
@@ -81,14 +388,22 @@
SVN_ERR (svn_client_revprop_list (&proplist,
URL, &(opt_state->start_revision),
&rev, ctx, pool));
-
- SVN_ERR
- (svn_cmdline_printf (pool,
+
+ if (opt_state->xml)
+ {
+ begin_xml_proplist (&sb, FALSE, pool);
+ SVN_ERR (group_props_xml (proplist, URL, sb, pool));
+ SVN_ERR (end_xml_proplist (sb, FALSE, pool));
+ }
+ else
+ {
+ SVN_ERR (svn_cmdline_printf (pool,
_("Unversioned properties on revision %ld:\n"),
rev));
- SVN_ERR (svn_cl__print_prop_hash
- (proplist, (! opt_state->verbose), pool));
+ SVN_ERR (iterate_prop_hash (proplist, print_prop_cl,
+ (void*)(! opt_state->verbose), pool));
+ }
}
else /* operate on normal, versioned properties (not revprops) */
{
@@ -98,7 +413,6 @@
{
const char *target = ((const char **) (targets->elts))[i];
apr_array_header_t *props;
- int j;
svn_boolean_t is_url = svn_path_is_url (target);
const char *truepath;
svn_opt_revision_t peg_revision;
@@ -120,23 +434,18 @@
SVN_ERR_ENTRY_NOT_FOUND,
SVN_NO_ERROR));
- for (j = 0; j < props->nelts; ++j)
+ if (opt_state->xml)
{
- svn_client_proplist_item_t *item
- = ((svn_client_proplist_item_t **)props->elts)[j];
- const char *name_local;
+ begin_xml_proplist (&sb, (props->nelts>1), pool);
+ SVN_ERR (iterate_prop_array (props, (print_path_style_t)is_url,
+ group_props_xml, sb, pool));
+ SVN_ERR (end_xml_proplist (sb, (props->nelts>1), pool));
+ }
+ else
+ SVN_ERR (iterate_prop_array (props, (print_path_style_t)is_url,
+ group_props_cl,
+ (void*)opt_state->verbose, pool));
- if (! is_url)
- name_local = svn_path_local_style (item->node_name->data,
- subpool);
- else
- name_local = item->node_name->data;
-
- SVN_ERR (svn_cmdline_printf(subpool, "Properties on '%s':\n",
- name_local));
- SVN_ERR (svn_cl__print_prop_hash
- (item->prop_hash, (! opt_state->verbose), subpool));
- }
}
svn_pool_destroy (subpool);
}
Index: cmdline/main.c
===================================================================
--- cmdline/main.c (revision 15395)
+++ cmdline/main.c (working copy)
@@ -548,7 +548,7 @@
" 1. Lists versioned props. If specified, REV determines in which\n"
" revision the target is first looked up.\n"
" 2. Lists unversioned remote props on repos revision.\n"),
- {'v', 'R', 'r', 'q', svn_cl__revprop_opt, SVN_CL__AUTH_OPTIONS,
+ {'v', 'R', 'r', 'q', svn_cl__revprop_opt, svn_cl__xml_opt, SVN_CL__AUTH_OPTIONS,
svn_cl__config_dir_opt} },
{ "propset", svn_cl__propset, {"pset", "ps"},
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jul 23 16:19:38 2005