Bug 75796 - [TAGGEDPDF] Accomodate for future extension of poppler_structure_element_get_text()
Summary: [TAGGEDPDF] Accomodate for future extension of poppler_structure_element_get_...
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: glib frontend (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-05 13:18 UTC by Adrian Perez de Castro
Modified: 2014-03-10 08:57 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
[PATCH] glib: Use flags argument in poppler_structure_element_get_text() (5.22 KB, patch)
2014-03-05 13:18 UTC, Adrian Perez de Castro
Details | Splinter Review
[PATCH v2] glib: Use flags argument in poppler_structure_element_get_text() (5.35 KB, patch)
2014-03-06 08:53 UTC, Adrian Perez de Castro
Details | Splinter Review

Description Adrian Perez de Castro 2014-03-05 13:18:35 UTC
Created attachment 95165 [details] [review]
[PATCH] glib: Use flags argument in poppler_structure_element_get_text()

Instead of accepting a boolean argument to enable recursive text extraction,
use a flags value argument to poppler_structure_element_get_text(). Text
extraction may add features in the future (for example, allowing using
alternate text as replacement for inline figures), and this will allow
to extend the method without introducing ABI or API breakage.
Comment 1 Carlos Garcia Campos 2014-03-06 08:17:20 UTC
Comment on attachment 95165 [details] [review]
[PATCH] glib: Use flags argument in poppler_structure_element_get_text()

Review of attachment 95165 [details] [review]:
-----------------------------------------------------------------

::: glib/poppler-structure-element.cc
@@ +666,4 @@
>  /**
>   * poppler_structure_element_get_text:
>   * @poppler_structure_element: A #PopplerStructureElement
> + * @flags: A #PopplerStructureGetTextFlags value, or %0 to disable

Yes, this is much better than a boolean parameter, but please, use a value in the enum instead of this 0 here. Something like POPPLER_STRUCTURE_GET_TEXT_NONE
Comment 2 Adrian Perez de Castro 2014-03-06 08:53:54 UTC
Created attachment 95214 [details] [review]
[PATCH v2] glib: Use flags argument in poppler_structure_element_get_text()

New version of the patch which adds the POPPLER_STRUCTURE_GET_TEXT_NONE
value to the flags enum.
Comment 3 Carlos Garcia Campos 2014-03-10 08:57:06 UTC
Pushed thanks


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.