On Wed, 2005-05-04 at 18:12 +0200, Peter N. Lundblad wrote:
> > +{
> > + svn_stringbuf_t *sb = svn_stringbuf_create ("", pool);
> > + const char *rev_str = SVN_IS_VALID_REVNUM (revision)
> > + ? apr_psprintf (pool, "%ld", revision)
> > + : " -";
>
> Don't use " -" to denote "unknown revision". Make the revision
> attribute optional instead. (Or follow my suggestion in another mail
> to follow the list.dtd style.
>
>
> > +
> ...
> > +}
> > +
> I think it is cleaner to create an blame-receiver_xml, instead of
> intermixing the XML handling with the traditional output.
if implemented blame_recevier_xml will be a duplication of blame_recevier.
Do U have any strong reasons, why U suggest this
> > @@ -128,16 +208,28 @@
> > SVN_ERR (svn_stream_for_stdout (&out, pool));
> > bl.opt_state = opt_state;
> > bl.out = out;
> > -
> > +
> > subpool = svn_pool_create (pool);
> >
> > + /* If output is not incremental, output the XML header and wrap
> > + everything in a top-level element. This makes the output in
> > + its entirety a well-formed XML document. */
> > + if (opt_state->xml && ! opt_state->incremental)
> > + SVN_ERR (print_header_xml (pool));
> > + else
> > + if (opt_state->incremental)
> > + return svn_error_create (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> > + _("'incremental' option only valid in
> > XML "> > + "mode"));
>
> Bad indentation.
Sorry, not able to find bad indentation here, could U pin-point it
> > 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,10 @@
> > +<!-- XML DTD for Subversion command-line client output. -->
> > +
> > +<!-- For "svn blame" -->
> > +<!ELEMENT blame (target*)>
> > +<!ELEMENT target (entry*)>
> > +<!ATTLIST target path CDATA #REQUIRED> <!-- path or URL: character -->
>
> Does ": character" mean it is a single character? Of course not, but
> it is a little confusing. The data type is clear from the declaration,
> so you can just drop that.
>
Why not ": character(s)" ?, its better than removing data type
-Alexander Thomas (AT)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu May 5 09:06:30 2005