You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2005/04/29 11:24:24 UTC
Re: [PATCH] 'svn blame --xml' and fix for mutliple targets for svn
blame
Alexander Thomas wrote:
> [[[
> Patch to add XML output for 'svn blame' and to fix problem with multiple
> targets on svn blame
These are two separate new features or enhancements. Please provide two
separate patches.
I started to implement "svn blame --xml" a while back, and ran up against this
problem: XML output must be in UTF-8 encoding, while the file being blamed may
be in an incompatible encoding. Therefore it is not possible to simply insert
the lines of text from the file into the XML output. Either they need
re-coding, which is impractical, or they should be omitted and the XML just
contain the blame information but not the text itself. The latter solution
could be useful.
> * subversion/clients/cmdline/blame-cmd.c
> (print_blame_xml): New function for XML output
> (blame_receiver): Modified to check for XML option and process
> accordingly
> (print_header_xml): New function to print XML header
> (print_footer_xml): New function to print XML footer
> (svn_cl__blame): Modified for XML output
>
> * subversion/clients/cmdline/main.c
> (svn_cl__cmd_table[]): added --xml and --incremental options
> for blame
>
> * subversion/clients/cmdline/dtd/blame.dtd
> New dtd for blame --xml
There's no mention in your detailed change log of fixing a problem with
multiple targets.
>
> * subversion/tests/clients/cmdline/blame_tests.py
> (blame_in_xml): New test for blame --xml
> (blame_on_multiple_targets): New test to verify blame output
> for multiple targets
> (test_list): added blame_in_xml, blame_on_multiple_targets
> ]]]
> Index: subversion/clients/cmdline/blame-cmd.c
> ===================================================================
> --- subversion/clients/cmdline/blame-cmd.c (revision 14461)
> +++ subversion/clients/cmdline/blame-cmd.c (working copy)
> @@ -37,7 +38,67 @@
>
>
> /*** Code. ***/
> +
> static svn_error_t *
> +print_blame_xml (svn_stream_t *out,
> + apr_pool_t *pool,
> + const char *rev,
> + const char *author,
> + const char *time,
> + const char *line)
> +{
> + svn_stringbuf_t *sb = svn_stringbuf_create ("", pool);
> +
> + /* "<entry>" */
> + svn_xml_make_open_tag (&sb, pool, svn_xml_normal,
> + "entry", NULL);
> +
> + /* "<rev>xx</rev>" */
> + if (rev)
> + {
> + svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
> + "rev", NULL);
> + svn_xml_escape_cdata_cstring (&sb, rev, pool);
> + svn_xml_make_close_tag (&sb, pool, "rev");
> + }
> +
> + /* "<author>xx</author>" */
> + if (author)
> + {
> + svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
> + "author", NULL);
> + svn_xml_escape_cdata_cstring (&sb, author, pool);
> + svn_xml_make_close_tag (&sb, pool, "author");
> + }
> +
> + /* "<time>xx</time>" */
> + if (time)
> + {
> + svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
> + "time", NULL);
> + svn_xml_escape_cdata_cstring (&sb, time, pool);
> + svn_xml_make_close_tag (&sb, pool, "time");
> + }
> +
> + /* "<details>xx</details>" */
"details"? What does that mean?
> @@ -151,7 +269,20 @@
> /* Check for a peg revision. */
> SVN_ERR (svn_opt_parse_path (&peg_revision, &truepath, target,
> subpool));
> -
> + /* "<traget ...>" */
"traget"?
> Index: subversion/clients/cmdline/dtd/blame.dtd
> ===================================================================
> --- subversion/clients/cmdline/dtd/blame.dtd (revision 0)
> +++ subversion/clients/cmdline/dtd/blame.dtd (revision 0)
> @@ -0,0 +1,11 @@
> +<!-- XML DTD for Subversion command-line client output. -->
> +
> +<!-- For "svn blame" -->
> +<!ELEMENT blame (target*)>
> +<!ELEMENT target (entry*)>
> +<!ATTLIST target path CDATA #REQUIRED> <!-- file: character -->
"file: character"?
> +<!ELEMENT entry (rev, author, time?, details)>
> +<!ELEMENT rev (#PCDATA)> <!-- revision -->
> +<!ELEMENT author (#PCDATA)> <!-- author -->
> +<!ELEMENT time (#PCDATA)> <!-- date and time -->
> +<!ELEMENT details (#PCDATA)> <!-- deatils or content changed -->
"deatils"? "details or content changed"? What do you really intend this field
to contain?
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] 'svn blame --xml' and fix for mutliple targets for svn
blame
Posted by Alexander Thomas <al...@collab.net>.
On Fri, 2005-04-29 at 15:23 +0200, Peter N. Lundblad wrote:
> That being said, I think the XML output for svn status is moe important
> than blame.
I am working on it and will be ready in couple of days time.
>
> Regards,
> //Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] 'svn blame --xml' and fix for mutliple targets for svn
blame
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Fri, 29 Apr 2005, Alexander Thomas wrote:
> On Fri, 2005-04-29 at 12:24 +0100, Julian Foad wrote:
> > Alexander Thomas wrote:
> > I started to implement "svn blame --xml" a while back, and ran up against this
> > problem: XML output must be in UTF-8 encoding, while the file being blamed may
> > be in an incompatible encoding. Therefore it is not possible to simply insert
> > the lines of text from the file into the XML output. Either they need
> > re-coding, which is impractical, or they should be omitted and the XML just
> > contain the blame information but not the text itself. The latter solution
> > could be useful.
> why do we need to implement xml for remaining 3 informations (rev,
> author, date)?
Because that's the essential information that blame gives you. (To be
precise, the rev is essential, but you could give the commt info as well,
since it is available and would be expensive to get.
> is it worth doing ...
If the username contains spaces, it is not easy to parse the traditional
output.
If it was not for the encoding prlbem, we would include the file contents
as well, I think. An application needing this information can just do an
svn cat to get it.
That being said, I think the XML output for svn status is moe important
than blame.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] 'svn blame --xml' and fix for mutliple targets for svn
blame
Posted by Julian Foad <ju...@btopenworld.com>.
Alexander Thomas wrote:
> On Fri, 2005-04-29 at 12:24 +0100, Julian Foad wrote:
> why do we need to implement xml for remaining 3 informations (rev,
> author, date)?
Well, the revision is the only canonical piece of information. The author and
date are just some of the properties of the revision.
> is it worth doing ...
No, I don't really think it is worth doing unless and until somebody shows a
definite need for it.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] 'svn blame --xml' and fix for mutliple targets for svn
blame
Posted by Alexander Thomas <al...@collab.net>.
On Fri, 2005-04-29 at 12:24 +0100, Julian Foad wrote:
> Alexander Thomas wrote:
> > [[[
> > Patch to add XML output for 'svn blame' and to fix problem with multiple
> > targets on svn blame
>
> These are two separate new features or enhancements. Please provide two
> separate patches.
ok
> I started to implement "svn blame --xml" a while back, and ran up against this
> problem: XML output must be in UTF-8 encoding, while the file being blamed may
> be in an incompatible encoding. Therefore it is not possible to simply insert
> the lines of text from the file into the XML output. Either they need
> re-coding, which is impractical, or they should be omitted and the XML just
> contain the blame information but not the text itself. The latter solution
> could be useful.
why do we need to implement xml for remaining 3 informations (rev,
author, date)?
is it worth doing ...
> "traget"?
Sorry Spelling mistake :). it's "target"
> "deatils"? "details or content changed"? What do you really intend this field
> to contain?
It's for content change. I choose "details" for keeping it short.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org