señior ¿tyrtle? wrote:
> On 7/28/05 6:42 PM, "Julian Foad" <julianfoad@btopenworld.com> sneezed:
>
>> - the output from all targets to the "svn proplist" command are contained in
>>a single XML document (rather than one per explicit target), as with the other
>>commands that output XML.
>
> Wasn't it like that before?..or perhaps I don't fully understand this
> sentence.
I probably didn't say it clearly enough. By "explicit targets", I meant the
targets named on the command line or via the "--targets" option. By "all
targets" I meant all the individual files and directories that are going to
have their properties listed; if the command is recursive then this can include
many files and directories that were not explicitly given. The outer loop in
svn_cl__proplist was looping over the explicit targets; your patch was
constructing an XML document inside that loop.
>>The major stylistic change I made is:
>>
>> - I've removed one level of iterator function, as this made things simpler
>>in my opinion.
>
> At the cost of performance...in group_props you're checking the 'xml'
> boolean in a loop.
Checking a boolean and branching on the result is not an operation that takes
significant time in the context of formatting and printing a chunk of text.
> I also happen to think the code was more extensible with
> two iteration function...but I guess this is your call as a maintainer.
The iteration functions are quite a neat idea, and yes extensible. It was
indeed rather a subjective decision to choose to drop one of them, but it did
seem to be over-kill for the extent to which it was being used. These
functions are private, so extensibility without the need to modify them is not
such a relevant consideration.
If the iterators could be made as slick as the Python language's way:
for property in prop_array:
foo(property)
then I'd be ecstatic and we could use them all over the place.
>>There are more changes I think should be made:
>>
>> - When displaying rev-props, the target's "path" attribute should be omitted
>>because it is irrelevant.
>
> Wouldn't it be the url to the repository... I think that's hardly
> irrelevant.
You're right that it does print a URL even if a local path was specified on the
command line - but it's the URL to whatever file or dir was specified, and it's
that part in particular that I meant was "irrelevant". The canonical URL to
the repository would be relevant and could be useful.
> 'svn ls --xml' prints the url to the repository.
It doesn't when I try it on my Subversion source working copy:
~/src/subversion> svn ls --xml
<?xml version="1.0" encoding="utf-8"?>
<lists>
<list
path=".">
<entry
kind="file">
<name>BUGS</name>
[...]
Oh, do you mean if you give it the URL to the repository then it tells you it?
In that case, it doesn't need to tell you because you already knew it. Or,
to be more generous, I could say that although it would be convenient if
meta-data like that were included in the XML document, it isn't essential and
our other XML command outputs do not generally include any, just giving the
rough equivalent of what the plain text output displays.
Is that what you think is wrong? If so, presumably your principle would be
that the XML should be understandable without knowing the particular command
that produced it. In that case, do we need to add more information to the
other cases and types of XML output?
If we think meta-data like the repository URL should be included (and I am
beginning to think it should), then it would be best to look at all of the XML
command outputs we support and make a proposal that addresses what meta-data is
appropriate for each and all of them. That could be done before, during or
after this basic "proplist" support.
>> - Should probably remove the "value" element, leaving the property's value
>>as the sole content of the "property" element. You made a remark about putting
>>the value in a sub-element but I can't see a real benefit.
>
> Say property-inheritance is implemented sometime in the near future...
> <property>
> <value></value>
> <parent>something meaningful here</parent>
> </property>
Thanks - that's quite a good argument. I still expect that any such meta-data
that we might want to add to the property would probably be represented as an
attribute ... but just the fact that you came up with a plausible medium-term
addition so easily makes me realise that I might well be failing to anticipate
the sorts of additions we might want. OK, let it remain thus.
[...]
> I haven't worked on it in days. I'd be happy to do some more, but feel
> pretty strong about the previous two items, so maybe when they're resolved.
OK, let me/us know your thoughts on the XML meta-data (repository URL etc.)
issue, or whether we can address that issue separately.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jul 29 00:59:14 2005