[Poppler-bugs] [Bug 2951] evince case-sensitive find isn't

[bugzilla-daemon at freedesktop.org]

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

--- Comment #7 from Carlos Garcia Campos <carlosgc at gnome.org> 2012-01-08 02:31:48 PST ---
Comment on attachment 55270
  --> https://bugs.freedesktop.org/attachment.cgi?id=55270
a patch to add case sensitive search to poppler glib

Review of attachment 55270:
 --> (https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=2951&attachment=55270)
-----------------------------------------------------------------

Thanks for the patch!, it looks good to me in general, but I have a few
comments. You also need to update /glib/reference/poppler-sections.txt adding
the new symbols there. It would also be great if you add find options to the
find demo (glib/demo/find.c), but that might be a following patch.

Thanks!

::: glib/poppler-page.cc
@@ +853,4 @@
>  }
>  
>  /**
> + * poppler_page_find_text_full:

I would use poppler_page_find_text_with_options() for consistency with
poppler_page_render_for_printing_with_options() instead of _full(). Also,
instead of using boolean parameters I would use a single parameter using flags
something like:

GList *poppler_find_text_with_options (PopplerPage *page, const gchar *text,
PopplerFindOptions options);

That way if need to add new options in the future we don't need to break the
API, and we don't need to remember the position of every parameter.

@@ +860,5 @@
> + * immediately after the last find result; else starts looking at
> + * <xMin>,<yMin>.  If <stopAtBottom> is true, stops looking at the
> + * bottom of the page; else if <stopAtLast> is true, stops looking
> + * just before the last find result; else stops looking at
> + * <xMax>,<yMax>.

The body goes after the parameters not before. Parameters are referenced using
@ not <>. In this case, document every option in the PopplerFindOptions enum
declaration, and here say that it finds text with the given options.

@@ +873,1 @@
>   * Return value: (element-type PopplerRectangle) (transfer full): a #GList of #PopplerRectangle,

Add * Since: 0.20 since this will be new API added in poppler 0.20

@@ +940,5 @@
> +                            TRUE,   // stop_at_bottom
> +                            FALSE,  // start_at_last
> +                            FALSE,  // stop_at_last
> +                            FALSE,  // case_sensitive
> +                            FALSE); // backward

You could add an option POPPLER_FIND_DEFAULT to the enum containing the default
options and use it here.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.