After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 703616 - Inconsistent deprecations. GtkIconSet and other classes.
Inconsistent deprecations. GtkIconSet and other classes.
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
3.9.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2013-07-04 14:42 UTC by Kjell Ahlstedt
Modified: 2013-07-09 11:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch: Fix some inconsistent deprecations (5.06 KB, patch)
2013-07-05 13:56 UTC, Kjell Ahlstedt
accepted-commit_now Details | Review
patch: Continue the deprecation of GtkIconSet (3.33 KB, patch)
2013-07-05 13:58 UTC, Kjell Ahlstedt
accepted-commit_now Details | Review

Description Kjell Ahlstedt 2013-07-04 14:42:42 UTC
Many functions and some classes have been deprecated lately. I find the present
state of gtk+/gtk somewhat inconsistent.

https://git.gnome.org/browse/gtk+/commit/?id=aef9dca9d5e17985faa2b3659e031a5d5064149e
deprecated GtkIconFactory, GtkIconSet and GtkIconSource. But there are still
non-deprecated functions that take a GtkIconSet* or GtkIconSource* parameter.
One function with a GtkIconSet* parameter has even been added after GtkIconSet
was deprecated.

There are a few other inconsistencies as well.

In the following list of inconsistencies, "deprecated in xxx.h" means
"preceded by GDK_DEPRECATED_IN_x_y"; "deprecated in xxx.c" means there is
a comment such as "Deprecated: x.y: aaaaaaaaaaaaa".


   GtkIconSet (GtkImage, GtkStyleContext)

gtk_icon_set_render_icon_pixbuf() is deprecated in gtkiconfactory.c, but not
in gtkstylecontext.h, where it's declared.

gtk_icon_set_render_icon_surface() is new in 3.10, deprecated neither in
gtkiconfactory.c nor in gtkstylecontext.h.

gtk_image_get_icon_set() is deprecated neither in gtkimage.c nor in gtkimage.h.

gtk_style_context_lookup_icon_set() is deprecated in gtkstylecontext.c,
but not in gtkstylecontext.h.

   GtkIconSource (GtkStyleContext)

gtk_render_icon_pixbuf() takes a GtkIconSource* parameter. It is deprecated in
gtkstylecontext.c, but not in gtkstylecontext.h.

   GtkActionGroup

gtk_action_group_get_accel_group() and gtk_action_group_set_accel_group() are
deprecated in gtkactiongroup.c, but not in gtkactiongroup.h.

   GtkContainer

gtk_container_resize_children() is deprecated in gtkcontainer.h, but not in
gtkcontainer.c. (No gtk-doc comment in gtkcontainer.c.)

   GtkNotebook

gtk_notebook_get_tab_hborder() and gtk_notebook_get_tab_vborder() are
deprecated in gtknotebook.c, but not in gtknotebook.h.
Comment 1 Emmanuele Bassi (:ebassi) 2013-07-04 14:47:54 UTC
thanks for the bug report.

can you prepare a patch that keeps the deprecations in the gtk-doc comment block and the deprecation annotations in sync?
Comment 2 Kjell Ahlstedt 2013-07-04 15:17:39 UTC
Yes, I can do that, but I'm not certain exactly how you want it done.
Are these the correct things to do?

- When a function is deprecated only in the .h file, add a deprecation comment
  in the .c file.
- When a function is deprecated only in the .c file, add a deprecation
  annotation in the .h file.
- When a function is not deprecated, but takes a parameter of a deprecated
  class, deprecate it in both the .h and the .c files.
  Even for gtk_icon_set_render_icon_surface(), which would then be both new in
  3.10 and deprecated in 3.10.

I take it for granted that the first two items in this list are correct.
I don't feel as certain about the last one. Isn't it a bit odd to add a
function that is deprecated the moment it's added?
Comment 3 Emmanuele Bassi (:ebassi) 2013-07-04 15:46:10 UTC
(In reply to comment #2)
> Yes, I can do that, but I'm not certain exactly how you want it done.
> Are these the correct things to do?
> 
> - When a function is deprecated only in the .h file, add a deprecation comment
>   in the .c file.

correct.

> - When a function is deprecated only in the .c file, add a deprecation
>   annotation in the .h file.

correct, if there is a deprecation macro for that particular version.

> - When a function is not deprecated, but takes a parameter of a deprecated
>   class, deprecate it in both the .h and the .c files.

this is debatable, and needs to be discussed. you can hop on irc.gnome.org, in the #gtk+ channel to talk to the gtk developers.

>   Even for gtk_icon_set_render_icon_surface(), which would then be both new in
>   3.10 and deprecated in 3.10.
 
> I take it for granted that the first two items in this list are correct.
> I don't feel as certain about the last one. Isn't it a bit odd to add a
> function that is deprecated the moment it's added?

I think that particular case was a result of two independent branches landing one after the other; the gtk_icon_set_render_icon_surface() API was added to implement the GTK_IMAGE_ICON_SET source for the hi-dpi display, so it will most likely need to stay, and be immediately deprecated, or be made private.
Comment 4 Kjell Ahlstedt 2013-07-05 13:56:34 UTC
Created attachment 248458 [details] [review]
patch: Fix some inconsistent deprecations

This patch fixes the deprecations that were marked only in the .c file or only
in the .h file.
Comment 5 Kjell Ahlstedt 2013-07-05 13:58:53 UTC
Created attachment 248459 [details] [review]
patch: Continue the deprecation of GtkIconSet

This patch deprecates the remaining two functions taking a GtkIconSet parameter
that were not deprecated.

I have not asked on IRC. I hope someone can check the patch here, and decide
whether it shall be committed or not.

I usually work with the C++ bindings. I noticed the inconsistencies when I was
deprecating the corresponding APIs in gtkmm.
Comment 6 Kjell Ahlstedt 2013-07-07 17:36:53 UTC
CC-ing William Jon McCann who deprecated GtkIconSet.

Did you deliberately or accidentally _not_ deprecate gtk_image_get_icon_set()?
gtk_icon_set_render_icon_surface() was added after you deprecated GtkIconSet.
Comment 7 William Jon McCann 2013-07-07 19:32:33 UTC
In my opinion, all API that uses GtkIconSet should be deprecated. If I missed some it was my mistake and a correction would be appreciated.
Comment 8 Matthias Clasen 2013-07-09 03:43:00 UTC
Review of attachment 248458 [details] [review]:

ok
Comment 9 Matthias Clasen 2013-07-09 03:43:43 UTC
Review of attachment 248459 [details] [review]:

ok