On Wed, 2011-05-04 at 23:38 +0100, Vincent Sanders wrote:
> > Index: render/html_internal.h
> > ===================================================================
> > --- /dev/null 2009-04-16 19:17:07.000000000 +0100
> > +++ render/html_internal.h 2011-05-04 22:29:08.000000000 +0100
> > @@ -0,0 +1,103 @@
> > +/*
> > + * Copyright 2004 James Bursa <[email protected]>
>
> really? I think its 2011 and you added this file ;-) maybe its a move tho?
The contents of this file were entirely extracted from render/html.h
> > +
> > +/** Data specific to CONTENT_HTML. */
> > +typedef struct html_content {
[...]
> > +} html_content;
>
> gah make a decision on before or after doccomments within one struct
> ;-) and are we saying 0 or NULL for empty ponters?
Done, and NULL.
> no doccomments?
Indeed not -- they weren't any before, either. At some point, we should
go through and wholesale move comments for public APIs into the headers
where they belong.
> > Index: image/image.c
> > ===================================================================
> > --- /dev/null 2009-04-16 19:17:07.000000000 +0100
> > +++ image/image.c 2011-05-04 22:29:39.000000000 +0100
>
>
> > +
> > +nserror image_init(void)
> > +{
>
> comment? or at least a ref to the header?
Done.
> > +
> > +void image_fini(void)
>
> ditto
Done.
> > Index: content/content_factory.c
> > ===================================================================
> > --- /dev/null 2009-04-16 19:17:07.000000000 +0100
> > +++ content/content_factory.c 2011-05-04 22:30:17.000000000 +0100
>
> file comment?
Done.
> > +
> > +typedef struct content_handler_entry {
>
>
> > + struct content_handler_entry *next;
> > +
> > + lwc_string *mime_type;
> > + const content_handler *handler;
> > +} content_handler_entry;
>
>
> whassat for then? comment teh struct a bit? tbh i shall shut up about
> comments now, its boring
Done.
> > +
> > +static content_handler_entry *content_handlers;
> > +
> > +nserror content_factory_register_handler(lwc_string *mime_type,
> > + const content_handler *handler)
> > +{
> > + content_handler_entry *e;
>
> e? really? perhaps entry might have been a little less brief?
Done.
> > +
> > +content_type content_factory_type_from_mime_type(const char *mime_type)
> > +{
> > + const content_handler *handler;
> > + lwc_string *imime_type;
> > + lwc_error lerror;
> > + content_type type;
> > +
> > + lerror = lwc_intern_string(mime_type, strlen(mime_type), &imime_type);
> > + if (lerror != lwc_error_ok)
> > + return CONTENT_NONE;
> > +
> > + handler = content_lookup(imime_type);
> > + if (handler == NULL) {
> > + lwc_string_unref(imime_type);
> > + return CONTENT_NONE;
> > + }
> > +
> > + type = handler->type(imime_type);
> > +
> > + lwc_string_unref(imime_type);
> > +
> > + return type;
> > +}
>
>
> maybe? better as:
>
> content_type type = CONTENT_NONE;
>
> lerror = lwc_intern_string(mime_type, strlen(mime_type), &imime_type);
> if (lerror != lwc_error_ok)
> return CONTENT_NONE;
>
> handler = content_lookup(imime_type);
> if (handler != NULL) {
> type = handler->type(imime_type);
> }
>
> lwc_string_unref(imime_type);
>
> return type;
Yes, thanks.
> > error:
> > if (error == BINDING_BADENCODING) {
> > - LOG(("Bad encoding: %s", html->encoding ? html->encoding : ""));
> > + LOG(("Bad encoding: %s", c->encoding ? c->encoding : ""));
> > msg_data.error = messages_get("ParsingFail");
> > } else
> > msg_data.error = messages_get("NoMemory");
> >
> > - content_broadcast(c, CONTENT_MSG_ERROR, msg_data);
> > - return false;
> > + content_broadcast(&c->base, CONTENT_MSG_ERROR, msg_data);
> > +
> > + return NSERROR_NOMEM;
>
> always no memory?
Indeed not. Fixed.
> >
> > /**
> > @@ -2174,12 +2209,11 @@
> > */
> > hlcache_handle *html_get_favicon(hlcache_handle *h)
>
> kept the API even though the rest of favicon went?
For now, yes -- there are sufficient callsites that removing it is
somewhat invasive (and orthogonal to the point of this changeset)
Ta,
J.
