> 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.