[Poppler-bugs] [Bug 64821] [TAGGEDPDF] Expose the structure tree and attributes in poppler-glib

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=64821

--- Comment #51 from Carlos Garcia Campos <carlosgc at gnome.org> ---
Comment on attachment 90261
  --> https://bugs.freedesktop.org/attachment.cgi?id=90261
[PATCH v12 04/11] glib: Expose the document structure tree

Review of attachment 90261:
-----------------------------------------------------------------

::: 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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20140209/b735ce7c/attachment.html>