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 770278 - modernize example applications
modernize example applications
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Documentation
3.21.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-08-23 13:43 UTC by Mohammed Sadiq
Modified: 2016-09-10 13:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[patch] use G_DECLARE_FINAL_TYPE in applications (33.36 KB, patch)
2016-09-06 18:03 UTC, Mohammed Sadiq
none Details | Review
[patch] use G_DECLARE_FINAL_TYPE in applications (33.53 KB, patch)
2016-09-10 00:40 UTC, Mohammed Sadiq
accepted-commit_now Details | Review

Description Mohammed Sadiq 2016-08-23 13:43:33 UTC
The examples included in application* folders at https://git.gnome.org/browse/gtk+/tree/examples are still not using the full capabilities of latest gtk+ revisions.

I feel guilty for not reporting this bug early. Hope somebody (may be a newcomer) can fix this and get the changes into gtk+ 3.22.

1. Use G_DECLARE_*TYPE:
   this is available since glib 2.44 I think. Using G_DECLARE* would reduce a considerable number of loc, which shall be helpful to newcomers.

2. Use "EXAMPLEAPP_H" as header guards.
   Names beginning with underscores are reserved as per the C standards. so "#ifndef __EXAMPLEAPP_H" should be replaced.

3. replace "class" with "klass"
   In code like ".. example_app_class_init (ExampleAppClass *class)" The gtk+ convention seems to be using "klass" which shall be an easy fix.

Thank you.
Comment 1 Emmanuele Bassi (:ebassi) 2016-08-23 14:04:10 UTC
(In reply to Mohammed Sadiq from comment #0)
> The examples included in application* folders at
> https://git.gnome.org/browse/gtk+/tree/examples are still not using the full
> capabilities of latest gtk+ revisions.
> 
> I feel guilty for not reporting this bug early. Hope somebody (may be a
> newcomer) can fix this and get the changes into gtk+ 3.22.
> 
> 1. Use G_DECLARE_*TYPE:
>    this is available since glib 2.44 I think. Using G_DECLARE* would reduce
> a considerable number of loc, which shall be helpful to newcomers.

Sure.

Would you be able to submit a patch to implement this?

> 2. Use "EXAMPLEAPP_H" as header guards.
>    Names beginning with underscores are reserved as per the C standards. so
> "#ifndef __EXAMPLEAPP_H" should be replaced.

This is pretty pointless, considering that GTK itself uses __GTK_FOO_H__; the guards are namespaces, so unless the ISO C working group decides to add __EXAMPLE to the list of standard pre-processor symbols, there's no actual collision possible.

We could consider using `#pragma once` instead, since it seems to be supported by all the compilers we care about (gcc, clang, and msvc).

> 3. replace "class" with "klass"
>    In code like ".. example_app_class_init (ExampleAppClass *class)" The
> gtk+ convention seems to be using "klass" which shall be an easy fix.

We use 'klass' to avoid issues with C++ compilers, and mostly because of legacy. There's no actual syntactical reason why you can't name a C variable `class`.
Comment 2 Mohammed Sadiq 2016-08-23 15:12:52 UTC
(In reply to Emmanuele Bassi (:ebassi) from comment #1)

> > 1. Use G_DECLARE_*TYPE:
> >    this is available since glib 2.44 I think. Using G_DECLARE* would reduce
> > a considerable number of loc, which shall be helpful to newcomers.
> 
> Sure.
> 
> Would you be able to submit a patch to implement this?
> 

I wish someone else shall do this. This shall be a nice exercise for newcomers.

In case there isn't a patch in a fortnight, I shall try.

> > 2. Use "EXAMPLEAPP_H" as header guards.
> >    Names beginning with underscores are reserved as per the C standards. so
> > "#ifndef __EXAMPLEAPP_H" should be replaced.
> 
> This is pretty pointless, considering that GTK itself uses __GTK_FOO_H__;
> the guards are namespaces, so unless the ISO C working group decides to add
> __EXAMPLE to the list of standard pre-processor symbols, there's no actual
> collision possible.
> 
> We could consider using `#pragma once` instead, since it seems to be
> supported by all the compilers we care about (gcc, clang, and msvc).
> 

Many people have little knowledge about the standards. Many have a notion that adding 2 (or 1) underscores is the way for header guards, which may have collisions. Better to be avoided in code intended as guides. But of course you know way better than me, so the decision is yours.

Thank you.
Comment 3 Mohammed Sadiq 2016-09-06 18:03:36 UTC
Created attachment 334925 [details] [review]
[patch] use G_DECLARE_FINAL_TYPE in applications
Comment 4 Matthias Clasen 2016-09-09 22:24:38 UTC
Doesn't quite want to build here:

make[4]: Entering directory '/home/mclasen/Sources/gtk+/examples/application6'
  CC       exampleapp-main.o
  CC       exampleapp-exampleapp.o
In file included from /home/mclasen/gnome/include/glib-2.0/gobject/gobject.h:24:0,
                 from /home/mclasen/gnome/include/glib-2.0/gobject/gbinding.h:29,
                 from /home/mclasen/gnome/include/glib-2.0/glib-object.h:23,
                 from /home/mclasen/gnome/include/glib-2.0/gio/gioenums.h:28,
                 from /home/mclasen/gnome/include/glib-2.0/gio/giotypes.h:28,
                 from /home/mclasen/gnome/include/glib-2.0/gio/gio.h:26,
                 from ../../gdk/gdkapplaunchcontext.h:28,
                 from ../../gdk/gdk.h:32,
                 from ../../gtk/gtk.h:30,
                 from exampleapp.c:1:
exampleapp.c: In function ‘example_app_get_type’:
exampleapp.c:12:15: error: invalid application of ‘sizeof’ to incomplete type ‘ExampleApp {aka struct _ExampleApp}’
 G_DEFINE_TYPE(ExampleApp, example_app, GTK_TYPE_APPLICATION);
               ^
/home/mclasen/gnome/include/glib-2.0/gobject/gtype.h:1968:48: note: in definition of macro ‘_G_DEFINE_TYPE_EXTENDED_BEGIN’
                                        sizeof (TypeName), \
                                                ^~~~~~~~
/home/mclasen/gnome/include/glib-2.0/gobject/gtype.h:1590:43: note: in expansion of macro ‘G_DEFINE_TYPE_EXTENDED’
 #define G_DEFINE_TYPE(TN, t_n, T_P)       G_DEFINE_TYPE_EXTENDED (TN, t_n, T_P, 0, {})
                                           ^~~~~~~~~~~~~~~~~~~~~~
exampleapp.c:12:1: note: in expansion of macro ‘G_DEFINE_TYPE’
 G_DEFINE_TYPE(ExampleApp, example_app, GTK_TYPE_APPLICATION);
 ^~~~~~~~~~~~~
Comment 5 Mohammed Sadiq 2016-09-10 00:40:59 UTC
Created attachment 335222 [details] [review]
[patch] use G_DECLARE_FINAL_TYPE in applications

G_DECLARE_FINAL_TYPE was introduced in glib 2.44.
We shall use that now so that lots of boilerplate code can
be reduced.
Comment 6 Matthias Clasen 2016-09-10 02:05:04 UTC
Review of attachment 335222 [details] [review]:

this works fine, and looks ok from a cursory look