callbacks.md

Callbacks Handlers

• Signal connections should be done using GtkBuilder/Glade where possible. If you're moving any manual GUI building code to the GtkBuilder file, you should ensure to move the signal connections there where possible (ex. where you don't need special user_data that cannot be set through Glade).

• If you're making a callback handler for any non-GtkBuilder-related signals place the handler nearby (ex. directly above) the function that uses it. For signal handlers connected using GtkBuilder that don't relate specifically to any existing code, place the handler functions in callbacks.c. If they are remotely related to the code in some file, but no particular function(s), place them together in a section of the relevant source file.

• Callback handlers should not contain complex logic or lots of code at all. Generally a callback handler will contain no more than a single function call to the actual function that performs the needed actions.

• Never call a callback handler explicitly in code. If you need this same functionality in other parts of the code, it's a clear indication that the handler contains too much code. Factor the common code out into a separate function and call it from the handler and wherever else needs to.

• Never add "public" declarations to any headers for callback handlers, this should never be needed if the other guidelines are followed, although there exists many such declarations in callbacks.h due to legacy use of Glade2.

• Callback handlers should be named like this:

    on_ [widget name] _ [signal_name]
  

  Regardless if their related to GtkBuilder at all, just for consistency and to make it easier to find/navigate the functions. Some examples are:

    Widget Name    Signal Name          Handler Name
    ===========    ===========          ============
    menuitem1      activate             on_menuitem1_activate
    BlahBlah       clicked              on_BlahBlah_clicked
    some-other     destroy              on_some_other_destroy
  

  This makes it easy to find handlers in the code based on widget names and also makes it very clear which signal is being handled by just looking at the function name.

• If a signal is connected using GtkBuilder/Glade, make sure the signal handler function contains a "G_MODULE_EXPORT" to ensure that the symbol will be exported and visible when GtkBuilder scans the module for symbols to connect to. Forgetting this will almost surely cause the signal handler not to be found/work on Windows.

• Don't use G_MODULE_EXPORT on any signal handlers that aren't connected using GtkBuilder as this needlessly exports private symbols on some platforms. All callback handlers that are connected directly in code should be static functions, local the relevant source file.

Although if signal handlers are only connected using Glade and don't contain any real/business code as mentioned, I could see some logic to always putting the handlers in callbacks.c (ie. always know where to look, no random seemingly unrelated/unused functions mixed in with the other sources, etc.)

Why have a callback handler call a function that does what the handler should do? That just adds spaghetti if the functionality isn't used anywhere else. If the code is only for the purpose of the callback you might as well do it in the callback (so long as it doesn't need globals or access to local functionality in another module).

Agree that callback handlers that do something that is used elsewhere should call a separate function doing the work, so other users call a sane function, not one with a void* user_data. This supports your "don't call a callback handler" rule.

Agree that the guy who moved Geany from Glade 2 to Glade 3 didn't finish cleaning up ;-P

Its perhaps a little strong on "no code in callbacks" as I said above, but otherwise I generally agree with the intent summarised as:

1. Don't call callbacks from code, use functions with proper prototypes not void*. These functions should be located in the relevant module.
2. Put all callbacks in callbacks.c (since you won't call them, see rule 1) so they can be found easily.
3. Callback functions should do just what the callback is meant to do, using only public functions from other modules. If it can't, move the functionality into the relevant module and call it from the callback.

The reason is to decouple the it along the lines of the UI hooks that call back into that application (the callback handlers being the necessary boilerplate to make this happen), and actual code that does stuff. The callback handlers are just necessary glue to bind these two things together, and they make lousy functions since they have fixed, often not useful arguments and incorrect or useless return types and are sort of bound to a particular widget/object/action whatever.

For local/non-Glade/trivial signals/handlers it's probably less important to have the separation though, if the signal handler prototype makes any sense or doesn't matter, I was mostly referring to pure GUI widget callbacks that just fire an action (ex. save/open/quit/etc).

Agree.

Agreed but:

1. I'd rather keep all Glade-only callbacks in callbacks.c than move them to the "most closely related place", because it makes it easier to know where to look for a Glade callback, and doesn't bloat the module code for no particular reason. As you both said, Glade callbacks are simply glue between UI and code, so a glue file seems good enough for me.
2. Although definitely not the norm, there are few callbacks that are legitimately used from different locations, like on_motion_event() that is used to implement the autofocus feature. It's used in Glade, editor, toolbar and VTE, and although there could be a UI function for that a large part of its feature is to be a callback returning FALSE, reimplementing it all around might not be so nice. We could add a UI function like ui_widget_track_focus() or something, but then either we have a separate callback for Glade or we need to manually do it where Glade currently does it.

Hum, also the toolbar code could maybe legitimately use Glade's callbacks, as it's merely the same thing, not sure.

I don't mean to don't use the handler (ie. connect to it) many times, I mean don't call it directly. I just search in Geany for "on_motion_event" and this function is connected to many times, but it's never called explicitly, and it's contents are completely trivial. For an example of what I mean (and that breaks most of the guidelines above), see "on_exit_clicked".

I was referring to this point:

• Never add "public" declarations to any headers for callback handlers, this should never be needed if the other guidelines are followed, although there exists many such declarations in callbacks.h due to legacy use of Glade2.

Ah, my bad, I was thinking mostly of connections by Glade in the original wording of that...which still sort of applies here because in all uses of on_motion_event except maybe in vte.c, the manual GUI building code should probably be moved into GtkBuilder file along with signal connections :) but yeah, in such cases of manual C code that hasn't yet been or won't be ported to Glade, it'd be useful to add a declaration to a header.

P.S. I'll never use Gist again for this, it sucks :)

make a Github issue :)