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

Re: your mail

From: Peter N. Lundblad <peter_at_famlundblad.se>
Date: 2005-05-05 21:00:32 CEST

On Wed, 4 May 2005 alexander@collab.net wrote:

> 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.
> blame_recevier_xml will be a duplication of blame_recevier. Do U have
> any strong reasons, why U suggest this
>
If you rewrite you're XML printing routine to be a blame_receiver_xml,
then you'll see that that function and the current blame_receiver don't
have much in common. They output completely different formats of the most
things. I think it is cleanre to separate those two things.

> > > @@ -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
>
Now it is screwed up by mail clients, but I think the " was below the (,
something like
_("first part "
"second part");
It should be
_("first part "
  "second part");

I could be wrong here; please ignore if so. Note that this usually
happens when you mark strings for translation and forget to reindent the
following lines.

> > > 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
>
That's what CDATA means. It's like saying
int i; /* counter: integer */
in C:-)

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu May 5 20:53:10 2005

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.