[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] issue #2069 - "svn status" in xml mode - v6

From: Peter N. Lundblad <peter_at_famlundblad.se>
Date: 2005-06-01 23:16:01 CEST

On Wed, 1 Jun 2005, Alexander Thomas wrote:

> Version 6 of the patch for svn status --xml is attached.
>
Below are some review comments for further patches. I fixed them, since
they were only stylistic and comment stuff. Committed in r 14921.

Thanks,
//Peter

Index: subversion/clients/cmdline/status.c
===================================================================
--- subversion/clients/cmdline/status.c (revision 14774)
+++ subversion/clients/cmdline/status.c (working copy)
@@ -24,8 +24,10 @@
 #include "svn_cmdline.h"
 #include "svn_wc.h"
 #include "svn_path.h"
+#include "svn_xml.h"
+#include "svn_time.h"
 #include "cl.h"
-
+#include "svn_private_config.h"

 /* Return the single character representation of STATUS */
 static char
@@ -51,6 +53,32 @@
     }
 }

+
+/* Return the detailed string representation of STATUS */
+static const char*

Space before *.

+generate_status_desc (enum svn_wc_status_kind status)
+{

+
+svn_error_t *
+svn_cl__print_status_xml (const char *path,
+ svn_wc_status2_t *status,
+ apr_pool_t *pool)
+{
+ svn_stringbuf_t *sb = svn_stringbuf_create ("", pool);
+
+ if (status->text_status == svn_wc_status_none
+ && status->repos_text_status == svn_wc_status_none)
+ return SVN_NO_ERROR;
+
+ svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "entry",
+ "path", svn_path_local_style (path, pool), NULL);
+
+ apr_hash_t *att_hash = svn_xml_make_att_hash (NULL, pool);

C89 doesn't allow declarations in the middle of a block. GCC does
(for example), so it is easy to introduce these kinds of portability
errors. I'm also replacing svn_xml_make_att_hash with
apr_hash_make, since the former is meant to be used when you have a
list of attributes to populate the hash with.

+ /* If lock_owner is NULL, assume WC is corrupt. */
+ if (status->entry->lock_owner)
+ {
+ svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
+ "owner", NULL);
+ svn_xml_escape_cdata_cstring (&sb, status->entry->lock_owner,
+ pool);
+ svn_xml_make_close_tag (&sb, pool, "owner");
+ }
+ else
+ return svn_error_createf (SVN_ERR_WC_CORRUPT, 0,
+ _("'%s' has lock token, but no lock owner\n"),
+ svn_path_local_style (path, pool));
+

INdentation, and error messages should not be terminated by a newline.

+ if (status->repos_lock->expiration_date != 0)
+ {
+ svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
+ "expires", NULL);
+ svn_xml_escape_cdata_cstring (&sb,
+ svn_time_to_cstring (status->repos_lock->expiration_date,
+ pool), pool);

Indentation.

+/* Prints XML target */
+static svn_error_t *
+print_start_target_xml (apr_pool_t *pool, const char *target)

pool argument should go last (exceptions are mostly variadic
functions). (I expanded the docstring.)

+/* Prints XML header */

All arguments should be mentioned in the docstring, even pool.

+static svn_error_t *
+print_header_xml (apr_pool_t *pool)

+/* Prints XML footer */

Same.

+static svn_error_t *
+print_footer_xml (apr_pool_t *pool)
+{
+ svn_stringbuf_t *sb = svn_stringbuf_create ("", pool);
+
+ svn_xml_make_close_tag (&sb, pool, "status");
+
+ return svn_cl__error_checked_fputs (sb->data, stdout);
+}
+
+/* Prints </target> and the revision in repository against which
+ * status was checked */

Mention arguments by name and the REPOS_REV is only printed if it is
valid. See the rewritten docstring in the commit.

+ if (opt_state->xml)
+ SVN_ERR (print_start_target_xml (

Misplaced paren.

+ pool, svn_path_local_style (target, pool)));
+
+ SVN_ERR (svn_client_status2 (&repos_rev, target, &rev, print_status, &sb,
                                    opt_state->nonrecursive ? FALSE : TRUE,
                                    opt_state->verbose,
                                    opt_state->update,
                                    opt_state->no_ignore,
                                    opt_state->ignore_externals,
                                    ctx, subpool));
+ if (opt_state->xml)
+ SVN_ERR(print_finish_target_xml (repos_rev, pool));

Missing space before paren.

+ output, error = svntest.actions.run_and_verify_svn (None, None, [],
+ 'status', file_path, '--xml', '-u')

Long line.

+

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jun 1 23:07:04 2005

This is an archived mail posted to the Subversion Dev mailing list.