You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Alexander Thomas <al...@collab.net> on 2005/08/02 06:59:54 UTC
Re: [PATCH] 'svn info --xml' - v4
On Tue, 2005-08-02 at 00:06 +0100, Julian Foad wrote:
> Alexander Thomas wrote:
> >
> > * subversion/clients/cmdline/dtd/info.dtd
> > New dtd for info --xml.
>
> This DTD file is missing from the patch this time. Please could you send it.
Sorry, I missed out. Here I am posting an new version of the patch,
after changing APR_OFF_T_FMT to "%ld" along with the info.dtd file.
Regards
-Alexander Thomas(AT)
Re: [PATCH] 'svn info --xml' - v5
Posted by Julian Foad <ju...@btopenworld.com>.
Julian Foad wrote:
>
> So, please find attached my version of your patch after making all of
> the above changes.
>
> I think this is ready to commit as far I am concerned, but I would
> really like someone else to review particularly the format of the XML if
> possible. If there are no further concerns in the next two or three
> days I will commit this.
No comments seen, so I've committed this in r16014.
Thanks, Alexander.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] 'svn info --xml' - v5
Posted by Michael W Thelen <mi...@pietdepsi.com>.
Julian Foad wrote:
>>> So, please find attached my version of your patch after making all of
>>> the above changes.
>>
>> Just a ping... this patch doesn't look like it's been applied yet. Is
>> anyone else able to review it? If not, and if it hasn't been applied in
>> a day or two, I can file an issue for it so it doesn't get lost.
>
> I'm not around for another week or two. Thanks for keeping an eye on it.
No problem! I'll leave it alone then. :-)
--
Michael W Thelen
It is a mistake to think you can solve any major problems just with
potatoes. -- Douglas Adams
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] 'svn info --xml' - v5
Posted by Julian Foad <ju...@btopenworld.com>.
Michael W Thelen wrote:
> Julian Foad wrote:
>
>>So, please find attached my version of your patch after making all of
>>the above changes.
>
> Just a ping... this patch doesn't look like it's been applied yet. Is
> anyone else able to review it? If not, and if it hasn't been applied in
> a day or two, I can file an issue for it so it doesn't get lost.
I'm not around for another week or two. Thanks for keeping an eye on it.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] 'svn info --xml' - v5
Posted by Michael W Thelen <mi...@pietdepsi.com>.
Julian Foad wrote:
> So, please find attached my version of your patch after making all of
> the above changes.
>
> I think this is ready to commit as far I am concerned, but I would
> really like someone else to review particularly the format of the XML if
> possible. If there are no further concerns in the next two or three
> days I will commit this.
>
> Please say if you don't agree with something I said or did, or if you
> would like to make further changes.
Just a ping... this patch doesn't look like it's been applied yet. Is
anyone else able to review it? If not, and if it hasn't been applied in
a day or two, I can file an issue for it so it doesn't get lost.
--
Michael W Thelen
It is a mistake to think you can solve any major problems just with
potatoes. -- Douglas Adams
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] 'svn info --xml' - v5
Posted by Julian Foad <ju...@btopenworld.com>.
Alexander Thomas wrote:
>
> Patch to add XML output for 'svn info'.
I'm testing this patch against its DTD by running the following command in my
Subversion working copy and in my "sandbox" working copy (which has all sorts
of odd things in it):
$ svn info --xml -R * | xmllint --dtdvalid
~/src/subversion/subversion/clients/cmdline/dtd/info.dtd --noout -
I've also been looking at the other DTDs (blame, list, log, status) to see how
this compares with them, and looking at the source of the information
(svn_info_t) to see what it really means and which bits are related to each other.
In deciding which elements are optional and which are dependent on others, I
could do with some help from somebody who understands the client and WC better
than me. To me, it seems that nearly everything is optional, and it seems that
this has only been determined empirically by finding test cases in which some
bit of information is not present.
One further thing I'd like to see, but that can be done afterwards, is the DTDs
(all of them) better laid out, with replaceable entities naming data types like
"pathname", "username", "date", etc., even though all of those are just defined
as "(#PCDATA)"; and better comments that state both the meaning and the data
type (format) of each field.
[...]
> (svn_cl__node_kind_str): New prototype.
[...]
> (make_tagged_cdata): New function.
I decided to add these both as semi-public functions, and have committed them
already in r15545, the latter named "svn_cl__xml_tagged_cdata".
> Index: subversion/clients/cmdline/dtd/info.dtd
> ===================================================================
> --- subversion/clients/cmdline/dtd/info.dtd (revision 0)
> +++ subversion/clients/cmdline/dtd/info.dtd (revision 0)
> @@ -0,0 +1,47 @@
> +<!-- XML DTD for Subversion command-line client output. -->
> +
> +<!-- For "svn info" -->
> +<!ELEMENT info (entry*)>
> +
> +<!ELEMENT entry (url?, repository?, wc-info?, last-changed?, conflict?, lock?)>
> +<!-- path: local path -->
> +<!-- kind: path type -->
> +<!-- revision number of wc or repos path/url-->
> +<!ATTLIST entry
> + path CDATA #REQUIRED
> + kind (file | directory) #REQUIRED
I've changed "directory" to "dir" for consistency with "svn list --xml".
> + revision CDATA #REQUIRED
> +>
> +<!ELEMENT url (#PCDATA)> <!-- repository url -->
> +
> +<!ELEMENT repository (root?, uuid)>
It seems that the UUID needs to be optional. I have some directories that have
both root and uuid, and some with only root, and some with only uuid.
> +<!ELEMENT root (#PCDATA)> <!-- root: repository URL -->
> +<!ELEMENT uuid (#PCDATA)> <!-- uuid: repository uuid -->
> +
> +<!ELEMENT wc-info (schedule?, copy-from-url?, copy-from-rev?, checksum?)>
> +<!-- schedule: applies only for wc -->
Hmm. All of the "wc-info" fields apply only to a wc.
> +<!ELEMENT schedule (#PCDATA | normal | add | delete | replace | none)*>
This implies mixed content. I've changed this to:
<!ELEMENT schedule (#PCDATA)> <!-- normal | add | delete | replace | none -->
> +<!ELEMENT copy-from-url (#PCDATA)>
> +<!ELEMENT copy-from-rev (#PCDATA)>
The code was writing "copied-from-rev". I've fixed the code.
> +<!ELEMENT checksum (#PCDATA)>
> +
> +<!ELEMENT last-changed (author, revision, date, text-updated?, prop-updated?)>
> +<!ELEMENT author (#PCDATA)> <!-- last changed author -->
> +<!ELEMENT revision (#PCDATA)> <!-- last changed revision -->
This "last-changed" element ought to be consistent with the "commit" element of
"status.dtd" and "list.dtd", even as far as being named "commit".
> +<!ELEMENT date (#PCDATA)> <!-- last changed date in iso format -->
> +<!ELEMENT text-updated (#PCDATA)> <!-- text last updated -->
> +<!ELEMENT prop-updated (#PCDATA)> <!-- properties last updated -->
Those last two elements are WC info, not last-changed info. See the definition
of svn_info_t for details.
> +<!ELEMENT conflict (prev-base-file, prev-wc-file?, cur-base-file, prop-file?)>
> +<!ELEMENT prev-base-file (#PCDATA)> <!-- previous base file -->
> +<!ELEMENT prev-wc-file (#PCDATA)> <!-- previous wc file -->
> +<!ELEMENT cur-base-file (#PCDATA)> <!-- current base file -->
> +<!ELEMENT prop-file (#PCDATA)> <!-- current properties file -->
This conflict information is also a subset of the WC information.
> +<!ELEMENT lock (token, owner, created, expires?, comment?)>
> +<!ELEMENT token (#PCDATA)> <!-- lock token -->
> +<!ELEMENT owner (#PCDATA)> <!-- lock owner -->
> +<!ELEMENT created (#PCDATA)> <!-- lock created: date & time in iso format -->
> +<!ELEMENT expires (#PCDATA)> <!-- lock expires: date & time in iso format -->
> +<!ELEMENT comment (#PCDATA)> <!-- lock comment -->
This "lock" element should be consistent with the one in "status.dtd"; it very
nearly is, but the order of the elements is different.
[...]
> + /* "<copied-from-rev> xx </copied-from-rev>" */
> + make_tagged_cdata (&sb, pool, "copied-from-rev",
I've corrected this from "copied-from-rev" to "copy-from-rev".
> + apr_psprintf (pool, "%ld", info->copyfrom_rev));
[...]
> @@ -76,26 +315,9 @@
> if (SVN_IS_VALID_REVNUM (info->rev))
> SVN_ERR (svn_cmdline_printf (pool, _("Revision: %ld\n"), info->rev));
>
> - switch (info->kind)
> - {
> - case svn_node_file:
> - SVN_ERR (svn_cmdline_printf (pool, _("Node Kind: file\n")));
> - break;
> -
> - case svn_node_dir:
> - SVN_ERR (svn_cmdline_printf (pool, _("Node Kind: directory\n")));
> - break;
> -
> - case svn_node_none:
> - SVN_ERR (svn_cmdline_printf (pool, _("Node Kind: none\n")));
> - break;
> -
> - case svn_node_unknown:
> - default:
> - SVN_ERR (svn_cmdline_printf (pool, _("Node Kind: unknown\n")));
> - break;
> - }
>
> + SVN_ERR (svn_cmdline_printf (pool, _("Node Kind: %s\n"),
> + svn_cl__node_kind_str (info->kind)));
As I said, we probably want those strings to remain localisable, while the XML
output is fixed, so we'll leave that part of the code as it was, and not call
the new "node_kind_str" function there.
So, please find attached my version of your patch after making all of the above
changes.
I think this is ready to commit as far I am concerned, but I would really like
someone else to review particularly the format of the XML if possible. If
there are no further concerns in the next two or three days I will commit this.
Please say if you don't agree with something I said or did, or if you would
like to make further changes.
- Julian