[Poppler-bugs] [Bug 58015] [PATCH] Add wrapper for Annot::setFlags

[bugzilla-daemon at freedesktop.org]

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

--- Comment #8 from Carlos Garcia Campos <carlosgc at gnome.org> ---
Comment on attachment 71198
  --> https://bugs.freedesktop.org/attachment.cgi?id=71198
patch: update the demo

Review of attachment 71198:
-----------------------------------------------------------------

Looks great, I have a just some minor comments

::: glib/demo/annots.c
@@ +694,5 @@
> +      case ANNOTS_FLAG_HIDDEN_COLUMN:
> +        flag_bit = POPPLER_ANNOT_FLAG_HIDDEN;
> +        break;
> +      default:
> +        flag_bit = POPPLER_ANNOT_FLAG_PRINT;

Instead of default, add the case ANNOTS_FLAG_PRINT_COLUMN: better, and add
default: doing nothing or with g_assert_not_reached().

@@ +699,5 @@
> +        break;
> +    }
> +
> +    if (fixed)
> +        flags = flags | flag_bit;

This could be flags |= flag_bit no?

@@ +701,5 @@
> +
> +    if (fixed)
> +        flags = flags | flag_bit;
> +    else
> +        flags = flags & (~flag_bit);

And this flags &= ~flag_bit

@@ +941,4 @@
>                                                   NULL);
>  
>      renderer = gtk_cell_renderer_toggle_new ();
> +    g_signal_connect (G_OBJECT (renderer), "toggled",

g_signal_connect expects a gpointer so no need to cast to GObject.

@@ +965,5 @@
>      renderer = gtk_cell_renderer_toggle_new ();
> +    g_signal_connect (G_OBJECT (renderer), "toggled",
> +                      G_CALLBACK (pgd_annots_flags_toggled),
> +                      (gpointer) demo);
> +    g_object_set_data (G_OBJECT (renderer), "column", GINT_TO_POINTER (ANNOTS_FLAG_PRINT_COLUMN));

I think it would be simpler to add three different callbacks and call a custom
method passing the column

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20121208/45a52759/attachment-0001.html>