<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW --- - [TAGGEDPDF] Expose the structure tree and attributes in poppler-glib" href="https://bugs.freedesktop.org/show_bug.cgi?id=64821#c51">Comment # 51</a> on <a class="bz_bug_link bz_status_NEW " title="NEW --- - [TAGGEDPDF] Expose the structure tree and attributes in poppler-glib" href="https://bugs.freedesktop.org/show_bug.cgi?id=64821">bug 64821</a> from <a class="email" href="mailto:carlosgc@gnome.org" title="Carlos Garcia Campos <carlosgc@gnome.org>"> Carlos Garcia Campos</a> <pre>Comment on <a href="attachment.cgi?id=90261" name="attach_90261" title="[PATCH v12 04/11] glib: Expose the document structure tree">attachment 90261</a> <a href="attachment.cgi?id=90261&action=edit" title="[PATCH v12 04/11] glib: Expose the document structure tree">[details]</a> <a href='page.cgi?id=splinter.html&bug=64821&attachment=90261'>[review]</a> [PATCH v12 04/11] glib: Expose the document structure tree Review of <a href="attachment.cgi?id=90261" name="attach_90261" title="[PATCH v12 04/11] glib: Expose the document structure tree">attachment 90261</a> <a href="attachment.cgi?id=90261&action=edit" title="[PATCH v12 04/11] glib: Expose the document structure tree">[details]</a> <a href='page.cgi?id=splinter.html&bug=64821&attachment=90261'>[review]</a>: ----------------------------------------------------------------- ::: glib/poppler-structure-element.cc @@ +44,5 @@ > + * and poppler_structure_get_child() to enumerate the top level elements. > + */ > + > +static PopplerStructureElement * > + _poppler_structure_element_new (PopplerDocument *document, StructElement *element); You should move the iter implementation to the bottom of the file and we don't need this prototype here. @@ +139,5 @@ > + * poppler_structure_element_iter_free (iter); > + * } > + * </programlisting></informalexample> > + * > + * Return value: (transfer full): a new #PopplerStructureElementIter , or %NULL if .... @@ +417,5 @@ > +PopplerStructureElementKind > +poppler_structure_element_get_kind (PopplerStructureElement *poppler_structure_element) > +{ > + g_return_val_if_fail (POPPLER_IS_STRUCTURE_ELEMENT (poppler_structure_element), POPPLER_STRUCTURE_ELEMENT_UNKNOWN); > + g_assert (poppler_structure_element->elem); Use g_return macros in public methods. @@ +419,5 @@ > +{ > + g_return_val_if_fail (POPPLER_IS_STRUCTURE_ELEMENT (poppler_structure_element), POPPLER_STRUCTURE_ELEMENT_UNKNOWN); > + g_assert (poppler_structure_element->elem); > + > + return _poppler_structelement_type_to_poppler_structure_element_kind (poppler_structure_element->elem->getType ()); This method is only used here, I think we can add th switch here instead. @@ +436,5 @@ > + */ > +gint > +poppler_structure_element_get_page (PopplerStructureElement *poppler_structure_element) > +{ > + g_return_val_if_fail (POPPLER_IS_STRUCTURE_ELEMENT (poppler_structure_element), POPPLER_STRUCTURE_ELEMENT_UNKNOWN); POPPLER_STRUCTURE_ELEMENT_UNKNOWN -> -1 @@ +454,5 @@ > + * @poppler_structure_element: A #PopplerStructureElement > + * > + * Checks whether an element is actual document content. > + * > + * Return value: Whether the element is content. %TRUE if ..., %FALSE otherwise. And this pattern in all other methods. @@ +524,5 @@ > + g_assert (poppler_structure_element->elem); > + > + GooString *string = poppler_structure_element->elem->getID (); > + gchar *result = string ? _poppler_goo_string_to_utf8 (string) : NULL; > + delete string; getID() doesn't copy the string @@ +546,5 @@ > + g_assert (poppler_structure_element->elem); > + > + GooString *string = poppler_structure_element->elem->getTitle (); > + gchar *result = string ? _poppler_goo_string_to_utf8 (string) : NULL; > + delete string; Ditto. @@ +574,5 @@ > + return NULL; > + > + GooString *string = poppler_structure_element->elem->getExpandedAbbr (); > + gchar *result = string ? _poppler_goo_string_to_utf8 (string) : NULL; > + delete string; Ditto. @@ +598,5 @@ > + g_assert (poppler_structure_element->elem); > + > + GooString *string = poppler_structure_element->elem->getLanguage (); > + gchar *result = string ? _poppler_goo_string_to_utf8 (string) : NULL; > + delete string; Ditto. @@ +626,5 @@ > + g_assert (poppler_structure_element->elem); > + > + GooString *string = poppler_structure_element->elem->getAltText (); > + gchar *result = string ? _poppler_goo_string_to_utf8 (string) : NULL; > + delete string; Ditto. @@ +656,5 @@ > + g_assert (poppler_structure_element->elem); > + > + GooString *string = poppler_structure_element->elem->getActualText (); > + gchar *result = string ? _poppler_goo_string_to_utf8 (string) : NULL; > + delete string; Ditto. ::: glib/reference/poppler-docs.sgml @@ +23,5 @@ > <xi:include href="xml/poppler-layer.xml"/> > <xi:include href="xml/poppler-media.xml"/> > <xi:include href="xml/poppler-movie.xml"/> > + <xi:include href="xml/poppler-structure.xml"/> > + <xi:include href="xml/poppler-structure-element.xml"/> Bad indentation here.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are on the CC list for the bug.</li> </ul> </body> </html>