You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Nuutti Kotivuori <na...@iki.fi> on 2002/05/01 20:35:11 UTC

[PATCH] add option for xml output in svn log

Implemented --xml option to 'svn log'. It makes the log output come
out as XML, with full dates. Example of the output:

,----[ svn log -r1:188 --xml README ]
| <log>
| <logentry
|    revision="1">
| <author>svn</author>
| <date>Thu 30 Aug 2001 23:24:14.966996 (day 242, dst 1, gmt_off -18000)</date>
| <msg>Initial import.</msg>
| </logentry>
| <logentry
|    revision="188">
| <author>kfogel</author>
| <date>Mon 1 Oct 2001 10:35:02.330226 (day 274, dst 1, gmt_off -18000)</date>
| <msg>* packages/, packages/rpm: new directories.
| 
| * README: Describe new top-level directory.
| 
| * packages/README: New file.
| 
| * packages/rpm/subversion.spec: Spec file for source rpm, contributed
| by David Summers &lt;david@summersoft.fay.ar.us&gt;.
| </msg>
| </logentry>
| </log>
`----

Changelog:

2002-05-01  Nuutti Kotivuori  <na...@iki.fi>

	* main.c: Add --xml option to svn_cl__options.
	(svn_cl__cmd_table): Add svn_cl__xml_opt to log.
	(main): Add handling for svn_cl__xml_opt.

	* cl.h: Add svn_cl__xml_opt to enum svn_cl__longopt_t.
	(svn_cl__opt_state_t): Add xml option.

	* log-cmd.c: Include svn_pools.h and svn_xml.h.
	(log_message_receiver_xml): New function
	(svn_cl__log): Use log_message_receiver_xml when specified.

Patch:

Index: ./subversion/clients/cmdline/cl.h
===================================================================
--- ./subversion/clients/cmdline/cl.h
+++ ./subversion/clients/cmdline/cl.h	Wed May  1 17:48:22 2002
@@ -52,6 +52,7 @@
   svn_cl__auth_username_opt,
   svn_cl__auth_password_opt,
   svn_cl__targets_opt,
+  svn_cl__xml_opt,
 } svn_cl__longopt_t;
 
 
@@ -99,6 +100,7 @@
   svn_stringbuf_t *extensions;
   /* Targets supplied from a file with --targets */
   apr_array_header_t *targets;
+  svn_boolean_t xml; /* output in xml */
 } svn_cl__opt_state_t;
 
 
Index: ./subversion/clients/cmdline/log-cmd.c
===================================================================
--- ./subversion/clients/cmdline/log-cmd.c
+++ ./subversion/clients/cmdline/log-cmd.c	Wed May  1 23:05:44 2002
@@ -31,6 +31,8 @@
 #include "svn_path.h"
 #include "svn_delta.h"
 #include "svn_error.h"
+#include "svn_pools.h"
+#include "svn_xml.h"
 #include "cl.h"
 
 
@@ -165,6 +167,94 @@
 }
 
 
+/* This implements `svn_log_message_receiver_t' aswell,
+   but outputs XML instead. */
+static svn_error_t *
+log_message_receiver_xml (void *baton,
+                          apr_hash_t *changed_paths,
+                          svn_revnum_t rev,
+                          const char *author,
+                          const char *date,
+                          const char *msg)
+{
+  struct log_message_receiver_baton *lb = baton;
+  /* New pool for every received message. */
+  apr_pool_t *subpool = svn_pool_create(lb->pool);
+  /* Collate whole log message into sb before printing. */
+  svn_stringbuf_t *sb = svn_stringbuf_create("", subpool);
+  char *revstr;
+
+  revstr = apr_psprintf(subpool, "%" SVN_REVNUM_T_FMT, rev);
+  /* <logentry revision="xxx"> */
+  svn_xml_make_open_tag (&sb, subpool, svn_xml_normal, "logentry",
+                         "revision", svn_stringbuf_create(revstr, subpool),
+                         NULL);
+
+  /* <author>xxx</author> */
+  svn_xml_make_open_tag (&sb, subpool, svn_xml_protect_pcdata, "author",
+                         NULL);
+  svn_xml_escape_nts(&sb, author, subpool);
+  svn_xml_make_close_tag (&sb, subpool, "author");
+
+  /* Print the full, uncut, date. This is machine output. */
+  /* <date>xxx</date> */
+  svn_xml_make_open_tag (&sb, subpool, svn_xml_protect_pcdata, "date",
+                         NULL);
+  svn_xml_escape_nts(&sb, date, subpool);
+  svn_xml_make_close_tag (&sb, subpool, "date");
+
+  if (changed_paths)
+    {
+      apr_hash_index_t *hi;
+      char *path;
+
+      /* <files> */
+      svn_xml_make_open_tag (&sb, subpool, svn_xml_normal, "files",
+                             NULL);
+      
+      for (hi = apr_hash_first(subpool, changed_paths);
+           hi != NULL;
+           hi = apr_hash_next(hi))
+        {
+          void *val;
+          char action;
+          char *actionstr;
+
+          apr_hash_this(hi, (void *) &path, NULL, &val);
+          action = (char) ((int) val);
+
+          actionstr = apr_psprintf(subpool, "%c",
+                                   (action == 'R' ? 'U' : action));
+          /* <path action="X">xxx</path> */
+          svn_xml_make_open_tag (&sb, subpool, svn_xml_protect_pcdata,
+                                 "path", "action",
+                                 svn_stringbuf_create(actionstr, subpool),
+                                 NULL);
+          svn_xml_escape_nts(&sb, path, subpool);
+          svn_xml_make_close_tag (&sb, subpool, "path");
+        }
+
+      /* </files> */
+      svn_xml_make_close_tag (&sb, subpool, "files");
+    }
+
+  /* <msg>xxx</msg> */
+  svn_xml_make_open_tag (&sb, subpool, svn_xml_protect_pcdata, "msg",
+                         NULL);
+  svn_xml_escape_nts(&sb, msg, subpool);
+  svn_xml_make_close_tag (&sb, subpool, "msg");
+  
+  /* </logentry> */
+  svn_xml_make_close_tag (&sb, subpool, "logentry");
+
+  printf ("%s", sb->data);
+
+  svn_pool_destroy(subpool);
+
+  return SVN_NO_ERROR;
+}
+
+
 svn_error_t *
 svn_cl__log (apr_getopt_t *os,
              svn_cl__opt_state_t *opt_state,
@@ -217,14 +307,51 @@
 
   lb.first_call = 1;
   lb.pool = pool;
-  SVN_ERR (svn_client_log (auth_baton,
-                           targets,
-                           &(opt_state->start_revision),
-                           &(opt_state->end_revision),
-                           opt_state->verbose,
-                           log_message_receiver,
-                           &lb,
-                           pool));
+  if(!opt_state->xml)
+    {
+      SVN_ERR (svn_client_log (auth_baton,
+                               targets,
+                               &(opt_state->start_revision),
+                               &(opt_state->end_revision),
+                               opt_state->verbose,
+                               log_message_receiver,
+                               &lb,
+                               pool));
+    }
+  else
+    {
+      svn_stringbuf_t *sb;
+
+      sb = NULL;
+
+      /* The header generation is commented out because it might not
+         be certain that the log messages are indeed the advertised
+         encoding, UTF-8. The absence of the header should not matter
+         to people processing the output, and they should add it
+         themselves if storing the output as a fully-formed XML
+         document. */
+      /* <?xml version="1.0" encoding="utf-8"?> */
+      /* svn_xml_make_header (&sb, pool); */
+
+      /* <log> */
+      svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "log",
+                             NULL);
+      printf ("%s", sb->data);
+      
+      SVN_ERR (svn_client_log (auth_baton,
+                               targets,
+                               &(opt_state->start_revision),
+                               &(opt_state->end_revision),
+                               opt_state->verbose,
+                               log_message_receiver_xml,
+                               &lb,
+                               pool));
+
+      sb = NULL;
+      /* </log> */
+      svn_xml_make_close_tag (&sb, pool, "log");
+      printf ("%s", sb->data); 
+   }
 
   return SVN_NO_ERROR;
 }
Index: ./subversion/clients/cmdline/main.c
===================================================================
--- ./subversion/clients/cmdline/main.c
+++ ./subversion/clients/cmdline/main.c	Wed May  1 17:53:05 2002
@@ -70,6 +70,7 @@
     {"password",      svn_cl__auth_password_opt, 1, "specify a password ARG"},
     {"extensions",    'x', 1, "pass \"ARG\" as bundled options to GNU diff"},
     {"targets",       svn_cl__targets_opt, 1, "pass contents of file \"ARG\" as additional args"},
+    {"xml",           svn_cl__xml_opt, 0, "output in xml"},
     {0,               0, 0}
   };
 
@@ -227,7 +228,7 @@
     "\n"
     "    svn log http://www.example.com/repo/project foo.c bar.c\n",
     {'r', 'D', 'v', svn_cl__targets_opt, svn_cl__auth_username_opt,
-     svn_cl__auth_password_opt} },
+     svn_cl__auth_password_opt, svn_cl__xml_opt} },
   
   { "merge", svn_cl__merge, {0},
     "merge:  apply the differences between two paths to a working copy path.\n"
@@ -978,6 +979,9 @@
                                      opt_arg);
             svn_handle_error (err, stderr, FALSE);
           }
+        break;
+      case svn_cl__xml_opt:
+        opt_state.xml = TRUE;
         break;
       case 'x':
         opt_state.extensions = svn_stringbuf_create(opt_arg, pool);

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] add option for xml output in svn log

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Nuutti Kotivuori <na...@iki.fi> writes:
> So this is just a very minor issue, but first, how should the
> indentation be in variables that are assigned lists - and secondly,
> should the (): format of changes be used for functions and
> structures only - or also for variables, enums, everything?

We generally use it for all affected symbols.

The support is svn-dev.el is known to be imperfect, it's just meant to
be a time-saver.  You still have to check its results by eye, though.

> XML. On the other hand, if they are bytes, then they really cannot
> be included in the XML as text, regardless of the escaping of
> characters - but encoded in some binary format.
> 
> But I guess these issues can wait until i18n is properly underway.

Agreed.

Running "make check" right now, about to commit the changes.

By the way, it helps to run "make check" even after adding a feature.
This change caused getopt_tests.py to fail (no big deal, it's easy to
fix, just letting you know for next time).

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] add option for xml output in svn log

Posted by Nuutti Kotivuori <na...@iki.fi>.
Karl Fogel wrote:
>> 	* main.c: Add --xml option to svn_cl__options.
>> 	(svn_cl__cmd_table): Add svn_cl__xml_opt to log.
>> 	(main): Add handling for svn_cl__xml_opt.
> 
> Typically we say "(svn_cl__options): Add --xml option.".
> This is only a minor nit, however.
> 
>> 	* cl.h: Add svn_cl__xml_opt to enum svn_cl__longopt_t.
>> 	(svn_cl__opt_state_t): Add xml option.
> 
> Same minor nit.

Actually - I went around and investigated this a bit more. I
created the Changelog entries with emacs, so I was wondering why
emacs did not find the name for these two.

It appears that it is using "beginning-of-defun" function to get to
the begging of the defun and then processing the function /
whatever name from there.

And like the documentation of "beginning-of-defun" says:
,----
| Normally a defun starts when there is an char with
| open-parenthesis syntax at the beginning of a line.  If
| `defun-prompt-regexp' is non-nil, then a string which matches
| that regexp may precede the open-parenthesis, and point ends up
| at the beginning of the line.
`----
only parenthesis at the beginning of the line are considered.

So, in the files is:
,----
| const apr_getopt_option_t svn_cl__options[] =
|   {
`----
versus
,----
| const svn_cl__cmd_desc_t svn_cl__cmd_table[] =
| {
`----
so only the latter case is caught by "beginning-of-defun".

Again, emacs indentation mode itself presents the former of the two
choices as "correct" indentation.

So this is just a very minor issue, but first, how should the
indentation be in variables that are assigned lists - and secondly,
should the (): format of changes be used for functions and
structures only - or also for variables, enums, everything?

>> +      /* The header generation is commented out because it might not
>> +         be certain that the log messages are indeed the advertised
>> +         encoding, UTF-8. The absence of the header should not matter
>> +         to people processing the output, and they should add it
>> +         themselves if storing the output as a fully-formed XML
>> +         document. */
>> +      /* <?xml version="1.0" encoding="utf-8"?> */
> 
> Interesting thought!  I guess this is the best solution we can do,
> isn't it?  Oh well.

Well the case runs a bit deeper here. I guess the real question is
that are log messages, by definition, text - or are they a bunch of
binary bytes? If they are text, then they probably would have a
character set - and a language, like someone on IRC pointed out -
and those would need to be handled somehow if they are dumped into
XML. On the other hand, if they are bytes, then they really cannot
be included in the XML as text, regardless of the escaping of
characters - but encoded in some binary format.

But I guess these issues can wait until i18n is properly underway.

> I'd like to just apply this, with the tweaks above, and then do the
> interface fix.  No need to resubmit, I'll take care of it.  Thanks for
> the patch.

Very nice, thanks.

-- Naked

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] add option for xml output in svn log

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Excellent patch!  A few comments below:

Nuutti Kotivuori <na...@iki.fi> writes:
> 2002-05-01  Nuutti Kotivuori  <na...@iki.fi>
> 
> 	* main.c: Add --xml option to svn_cl__options.
> 	(svn_cl__cmd_table): Add svn_cl__xml_opt to log.
> 	(main): Add handling for svn_cl__xml_opt.

Typically we say "(svn_cl__options): Add --xml option.".
This is only a minor nit, however.

> 	* cl.h: Add svn_cl__xml_opt to enum svn_cl__longopt_t.
> 	(svn_cl__opt_state_t): Add xml option.

Same minor nit.

> Index: ./subversion/clients/cmdline/cl.h
> ===================================================================
> --- ./subversion/clients/cmdline/cl.h
> +++ ./subversion/clients/cmdline/cl.h	Wed May  1 17:48:22 2002
> @@ -52,6 +52,7 @@
>    svn_cl__auth_username_opt,
>    svn_cl__auth_password_opt,
>    svn_cl__targets_opt,
> +  svn_cl__xml_opt,
>  } svn_cl__longopt_t;

Greg Stein pointed out the possibility of a long option "--format"

   $ svn --format xml log
   $ svn --format=xml log

and the "--xml" could/would be a synonym for it.  There might be other
formats in the future.  But then we thought, the point of xml is so
that svn doesn't need to know those other formats...  After all, both
the default (human-readable) log format and the xml format are machine
parseable.  If someone wants something else, they can just convert one
of those, especially given how cheap and ubiquitous xml parsers are
these days.

So, --xml it is. :-)

> +/* This implements `svn_log_message_receiver_t' aswell,
> +   but outputs XML instead. */
> +static svn_error_t *
> +log_message_receiver_xml (void *baton,
> +                          apr_hash_t *changed_paths,
> +                          svn_revnum_t rev,
> +                          const char *author,
> +                          const char *date,
> +                          const char *msg)
> +{
> +  struct log_message_receiver_baton *lb = baton;
> +  /* New pool for every received message. */
> +  apr_pool_t *subpool = svn_pool_create(lb->pool);
> +  /* Collate whole log message into sb before printing. */
> +  svn_stringbuf_t *sb = svn_stringbuf_create("", subpool);
> +  char *revstr;

This is not a problem with your patch, but rather with the
`svn_log_message_receiver_t' prototype.  It was written before we hit
on the right way of using apr pools.  It should (like other, more
modern functions) take a pool argument.  Its *caller* would choose
whether or not to keep passing the same pool, perhaps using
apr_pool_clear(), because the caller knows whether or not
log_message_receiver is being called in a loop.

So, mental note: before or after this patch is applied, we should
change that interface, and then adjust this code to not create its own
subpool anymore.

> +      /* <files> */
> +      svn_xml_make_open_tag (&sb, subpool, svn_xml_normal, "files",
> +                             NULL);

These might not just be files, could be dirs (property changes).
Suggest "paths" instead of "files".

> +      /* </files> */
> +      svn_xml_make_close_tag (&sb, subpool, "files");

And "paths" here too, of course.

> +  if(!opt_state->xml)
> +    {
> +      SVN_ERR (svn_client_log (auth_baton,
> +                               targets,
> +                               &(opt_state->start_revision),
> +                               &(opt_state->end_revision),
> +                               opt_state->verbose,
> +                               log_message_receiver,
> +                               &lb,
> +                               pool));
> +    }
> +  else

Might want to put a comment somewhere around here that we are in the
XML case.  I know it should be obvious to anyone reading the code, but
it can help people browsing backwards, or who jumped here in a tags
operation, etc.

> +    {
> +      svn_stringbuf_t *sb;
> +
> +      sb = NULL;
> +
> +      /* The header generation is commented out because it might not
> +         be certain that the log messages are indeed the advertised
> +         encoding, UTF-8. The absence of the header should not matter
> +         to people processing the output, and they should add it
> +         themselves if storing the output as a fully-formed XML
> +         document. */
> +      /* <?xml version="1.0" encoding="utf-8"?> */

Interesting thought!  I guess this is the best solution we can do,
isn't it?  Oh well.

> +      /* svn_xml_make_header (&sb, pool); */
> +
> +      /* <log> */
> +      svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "log",
> +                             NULL);
> +      printf ("%s", sb->data);
> +      
> +      SVN_ERR (svn_client_log (auth_baton,
> +                               targets,
> +                               &(opt_state->start_revision),
> +                               &(opt_state->end_revision),
> +                               opt_state->verbose,
> +                               log_message_receiver_xml,
> +                               &lb,
> +                               pool));

Yup, looks good!

Mental note (to self more than anyone else): svn_client_log() is where
the pool/subpool decision will be made, after we fix that interface.

I'd like to just apply this, with the tweaks above, and then do the
interface fix.  No need to resubmit, I'll take care of it.  Thanks for
the patch.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org