Re: [xml] xmlWriter's indent support: initial patch

On Mon, Dec 29, 2003 at 03:42:17PM -0300, Lucas Brasilino wrote:

      Here goes a initial patch (against CVS) to implement indent
support to xmlWriter API. Only code creating documents using:


      can be indented by now. I'll continue implementing to others
      This patch have tooked a while 'cause I've implemented in two
other different ways. This one is the third implementation, and looks
much clear for me :)

 Hum, you're doing something not allowed which is changing APIs:

-xmlNewTextWriterFilename(const char *uri, int compression)
+xmlNewTextWriterFilename(const char *uri, int compression, int indent)

xmlNewTextWriterFilename is a public function, it was released with a given
signature, and changing it is not proper. You must create a new function
to pass the extra argument, that's the drawback of building things pieces by
pieces in a library... The fact that you had to change the example
is a good indication that this can't fly as is :-) . The best is to make
an extra new public function xmlNewTextWriterIndentFilename() and get
xmlNewTextWriterFilename() to call it with indent = 0. The alternative 
would simply to have a function 
   int xmlTextWriterIndent(xmlTextWriterPtr writer, int indent);
I think it's more in line with ObjectOriented APIs and a better general API.

Also the general usage seems that the default indentation string is
"  " not " ", i.e.  two spaces, not one. I think I would change this
default. I would also provide a way to set the indentation like
   int xmlTextWriterSetIndent(xmlTextWriterPtr writer, const xmlChar *str);

Also I don't understand why xmlTextWriterWriteIndent() need to be made public.
What's the purpose ?

  I think the API need a bit of refactoring before going in for the reasons
I expose, this keep most of the patch code unchanged. I could do the
change or take a new patch from you, as you prefer...



Daniel Veillard      | Red Hat Network
veillard redhat com  | libxml GNOME XML XSLT toolkit | Rpmfind RPM search engine

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]