Re: [xml] Potential wrong usage of xmlIsID() in tree.c

[Rob Richards <rrichards ctindustries net>]

Kasimier Buchcik wrote:
> Hi,
>
> On Fri, 2006-02-24 at 14:57 +0100, Kasimier Buchcik wrote:
>
> [...]
>
>   
> > My intention is now just try to implement some prototypes explicitely, 
> > i.e., without trying to reuse the code base, then send it to the
> > list for discussion, then maybe commit and then we still can try to
> > eliminate redundant code and make the code base more flexible.
> > Is this OK with you all?
> >     
>
> I attached the current prototypes for setting/adding DOM attrs.
>
> Compared to the default code base, I mostly see the following
> differences (this is written from the DOM-viewpoint):
>
> 1) Adding an attr-node
>   a) we need a removed attr-node to be returned
>   b) adjustable IDness handling (in xmlDOMWrapRegisterIfId())
>
> 2) Setting attrs
>   a) DOM: "value is a simple string; it is not parsed as it is being
>            set"
>   b) same as 1b)
>
> Some other internal functions come with this, but they might not be
> needed; I just wanted to minimize the relevant code.
>   
A lot of the code can be elimintated using existing functionality, such 
as what's the different between the xmlGetProp functions and just 
xmlHasNsProp or xmlTreeNewPropRaw and xmlNewDocProp?

The one question I have regards the function signatures. I'm assuming 
that the ones with leading comments are the ones to be exposed 
(everything marked static right now). For the options parameter. Are 
options going to be used often? If not, it might make sense to make them 
part of the xmlDOMWrapCtxt structure.
Also, since the context is optional, it might make sense to pass it last 
(the functions then are very close to the existing libxml2 functions.
I would also eleminate the doc parameter in most cases. This should be 
able to be determined from the node being passed in (at least for these 
functions). Lastly, if the function is only returning a 0 or -1, would 
it make more sense to just return the actual attribute (again similar to 
the libxml2 function).
i.e.:
xmlAttrPtr
xmlDOMWrapSetAttr(xmlNodePtr owner,
         xmlNsPtr ns,
         const xmlChar *name, const xmlChar *value,
         xmlDOMWrapCtxtPtr ctxt)

Moving the context paramter is not a big deal as I see it is already 
passed first in other of the xmlDOM... functions. Just thought it would 
make the mapping to the existing similar functions easier if possible.

Rob