<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#c30">Comment # 30</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:aperez@igalia.com" title="Adrian Perez de Castro <aperez@igalia.com>"> Adrian Perez de Castro</a> <pre>(In reply to <a href="show_bug.cgi?id=64821#c26">comment #26</a>) > Comment on <a href="attachment.cgi?id=86684" name="attach_86684" title="[PATCH v8 11/15] glib: Expose inline attributes of structure elements">attachment 86684</a> <a href="attachment.cgi?id=86684&action=edit" title="[PATCH v8 11/15] glib: Expose inline attributes of structure elements">[details]</a> <a href='page.cgi?id=splinter.html&bug=64821&attachment=86684'>[review]</a> [review] > [PATCH v8 11/15] glib: Expose inline attributes of structure elements > > Review of <a href="attachment.cgi?id=86684" name="attach_86684" title="[PATCH v8 11/15] glib: Expose inline attributes of structure elements">attachment 86684</a> <a href="attachment.cgi?id=86684&action=edit" title="[PATCH v8 11/15] glib: Expose inline attributes of structure elements">[details]</a> <a href='page.cgi?id=splinter.html&bug=64821&attachment=86684'>[review]</a> [review]: > ----------------------------------------------------------------- > > ::: glib/poppler-structure-element.cc > @@ +784,5 @@ > > + GList *glist; > > + char buf[8]; > > + Guint flags; > > + Guint color; > > +}; > > I think this implementation could go to the core so that other frontends > could use it. Using the goo types (or C++ ones) instead of glib ones. > Frontends would only need to iterate a list to build their own list. For the moment I am going to provide an updated patch for this, in a later patch set I will move the SpanBuilder class to Poppler core. > @@ +798,5 @@ > > + * > > + * Return value: (transfer none) (element-type PopplerTextSpan): A #GList > > + * of #PopplerTextSpan structures. > > + */ > > +GList* > > GList* -> GList *. If we get a list from the core and we know its size > beforehand, maybe we could return an array instead of a GList. I will do this when moving SpanBuilder to Poppler core. > @@ +819,5 @@ > > + > > +/** > > + * poppler_text_span_is_fixed_width: > > + * @poppler_text_span: a #PopplerTextSpan > > + */ > > This is missing a bried description, Return value and Since tags. Also, I am renaming it to poppler_text_span_is_fixed_width_font(), for coherency with poppler_text_span_is_serif_font(). > @@ +839,5 @@ > > + > > +/** > > + * poppler_text_span_is_bols: > > + * @poppler_text_span: a #PopplerTextSpan > > + */ > > Ditto. Also, I am renaming it to poppler_text_span_is_bold_font(), for coherency with poppler_text_span_is_serif_font(). > ::: glib/poppler-structure-element.h > @@ +87,5 @@ > > + gchar *text; > > + gchar *font_name; > > + gchar *link_target; > > + guint flags; > > + guint color; /* 0x00RRGGBB */ > > We could conver this to a PopplerColor when building the span to make it > more convenient to use for users. > > […] > > Maybe we could make this private and add getters for all other members as > well. Sure thing.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are on the CC list for the bug.</li> </ul> </body> </html>