Alexander Thomas wrote:
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 13821)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -543,7 +543,10 @@
> svn_wc_notify_failed_lock,
>
> /** @since New in 1.2. Failed to unlock a path. */
> - svn_wc_notify_failed_unlock
> + svn_wc_notify_failed_unlock,
> +
> + /** The last notification in a status xml (including status on externals). */
> + svn_wc_notify_status_xml_completed,
Don't add a comma at the end of the list - it's invalid C'89 (though some
compilers accept it).
> } svn_wc_notify_action_t;
>
>
> Index: subversion/libsvn_client/status.c
> ===================================================================
> --- subversion/libsvn_client/status.c (revision 13821)
> +++ subversion/libsvn_client/status.c (working copy)
> @@ -324,8 +324,11 @@
>
> if (ctx->notify_func2 && update)
> {
> - svn_wc_notify_t *notify
> - = svn_wc_create_notify (path, svn_wc_notify_status_completed, pool);
> + svn_wc_notify_t *notify;
> + if ( *((svn_boolean_t*)status_baton))
No - that cast is ugly and wrong. This function can't assume anything about
the content of the argument "status_baton".
(I note that the function is already creating a little confusion by using a
type "struct status_baton" and an argument "status_baton" which is not of that
type.)
> + notify = svn_wc_create_notify (path, svn_wc_notify_status_xml_completed, pool);
> + else
> + notify = svn_wc_create_notify (path, svn_wc_notify_status_completed, pool);
> notify->revision = edit_revision;
> (ctx->notify_func2) (ctx->notify_baton2, notify, pool);
> }
> Index: subversion/clients/cmdline/notify.c
> ===================================================================
> --- subversion/clients/cmdline/notify.c (revision 13821)
> +++ subversion/clients/cmdline/notify.c (working copy)
[...]
> @@ -55,8 +56,10 @@
> {
> struct notify_baton *nb = baton;
> char statchar_buf[5] = " ";
> + const char *staton_rev;
> const char *path_local;
> svn_error_t *err;
> + svn_stringbuf_t *sb;
Where you want variables that are local to a small block of code, please
declare them within the small block of code.
>
> path_local = svn_path_local_style (n->path, pool);
>
> @@ -364,6 +367,26 @@
> svn_handle_error (n->err, stderr, FALSE);
> break;
>
> + case svn_wc_notify_status_xml_completed:
> + if (SVN_IS_VALID_REVNUM (n->revision))
> + sb = svn_stringbuf_create ("", pool);
I assume you also meant the following several statements to be subject to the
"if", so you need braces. Then you will be able to declare the two local
variables in this local block:
if (...)
{
svn_stringbuf_t *sb = svn_stringbuf_create ("", pool);
const char *staton_rev = apr_psprintf (pool, "%ld", n->revision);
...
}
> + staton_rev = apr_psprintf (pool, "%ld", n->revision);
> + /* "<status-against ...>" */
> + svn_xml_make_open_tag (&sb, pool, svn_xml_normal,
> + "status-against", NULL);
> +
> + /* "<rev>xx</rev>" */
> + svn_xml_make_open_tag (&sb, pool, svn_xml_protect_pcdata,
> + "rev", NULL);
> + svn_xml_escape_cdata_cstring (&sb, staton_rev, pool);
> + svn_xml_make_close_tag (&sb, pool, "rev");
> +
> + /* "</status-against>" */
> + svn_xml_make_close_tag (&sb, pool, "status-against");
> +
> + svn_cl__error_checked_fputs (sb->data, stdout);
> + break;
> +
> default:
> break;
> }
> Index: subversion/clients/cmdline/status-cmd.c
> ===================================================================
> --- subversion/clients/cmdline/status-cmd.c (revision 13821)
> +++ subversion/clients/cmdline/status-cmd.c (working copy)
[...]
> @@ -127,9 +176,15 @@
> opt_state->no_ignore,
> opt_state->ignore_externals,
> ctx, subpool));
> +
Try to avoid accidental changes to whitespace like here ...
> }
>
> svn_pool_destroy (subpool);
> -
... and here (your inserted code is unrelated to destroying the pool, so a
separating line is good, though deleting the two space characters in it would
be fine) ...
> + if (opt_state->xml)
> + {
> + if (! opt_state->incremental)
> + SVN_ERR (print_footer_xml (pool));
> + }
> +
... and that nearly-blank line contains a space.
> return SVN_NO_ERROR;
> }
> Index: subversion/tests/clients/cmdline/stat_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/stat_tests.py (revision 13821)
> +++ subversion/tests/clients/cmdline/stat_tests.py (working copy)
> @@ -817,6 +817,69 @@
> svntest.actions.run_and_verify_status(wc_dir, expected_status)
>
>
> +def status_in_xml(sbox):
> + "status output in XML format"
> +
> +
> + output, error = svntest.actions.run_and_verify_svn (None, None, [],
> + 'help', 'status')
> +## TODO: is it neccessary to check output for NULL before using it
No, I don't think so.
[...]
> + # TODO: Checks wheather output of the cmd 'status --xml' is in XML format
Isn't that what the code below does?
[...]
> + template = ["<?xml version=\"1.0\" encoding=\"utf-8\"?>\n",
> + "<status>\n",
> + "<entry\n",
> + " file=\"%s\">\n" % (file_path),
> + "<itemstatus>M</itemstatus>\n",
> + "<propstatus> </propstatus>\n",
> + "<locked> </locked>\n",
> + "<scheduled> </scheduled>\n",
> + "<switched> </switched>\n",
> + "<reposlock> </reposlock>\n",
> + "</entry>\n",
> + "</status>\n",
> + ]
> +
> +
> + output, error = svntest.actions.run_and_verify_svn (None, None, [],
> + 'status', wc_dir, '--xml')
> +
> + for i in range(0, len(output)):
> + if output[i] != template[i]:
> + raise svntest.Failure
> +
> #----------------------------------------------------------------------
I hope that helps. I haven't reviewed all of the patch.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Apr 27 19:01:40 2005