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

[Daniel Veillard <veillard redhat com>]

On Mon, Dec 29, 2003 at 03:42:17PM -0300, Lucas Brasilino wrote:
> Hi
>
>       Here goes a initial patch (against CVS) to implement indent
> support to xmlWriter API. Only code creating documents using:
>
> xmlNewTextWriterFilename()
> xmlTextWriterWriteComment()
> xmlTextWriterStartElement()
> xmlTextWriterEndElement()
> xmlTextWriterWriteString()
>
>       can be indented by now. I'll continue implementing to others
> one.
>       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:

xmlTextWriterPtr
-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
 doc/examples/testWriter.c
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...

    thanks,

Daniel

-- 
Daniel Veillard      | Red Hat Network https://rhn.redhat.com/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/