Re: PROPOSAL: revise GLib message system

[Owen Taylor <otaylor redhat com>]

Tim Janik <timj@gtk.org> writes:

> On 7 Aug 1998, Owen Taylor wrote:
> 
> > Objection, your honor!
> > 
> > I think defining g_warning (etc.) to do different things in different
> > places, is evil preprocessor magic, and should be avoided. All
> > the unprefixed g_* should be left to behave as is. (On the surface).
> > 
> > Then, add:
> > 
> >  g_info (GQuark who, GSeverity severity, message, ...)
> >  g_infov (GQuark who, GSeverity severity, message, va_list args)
> >  g_info_set_handler (GQuark who, GSeverity severity, GInfoHandler handler);
> > 
> > [ Maybe make GSeverity a flags field so the same handler could
> >   handle a set of warning? ]
> 
> you could just use multiple calls for such cases, e.g.
> typedef enum
> {
>   G_SEVERITY_FATAL,
>   G_SEVERITY_WARNING,
>   G_SEVERITY_INFO,
>   G_SEVERITY_DEBUG
> } GSeverityType;
> 
> g_info_set_handler (gtk_quark, G_SEVERITY_FATAL, gtk_info_handler_abort);
> g_info_set_handler (gtk_quark, G_SEVERITY_CRITICAL, gtk_info_handler_continue);
> g_info_set_handler (gtk_quark, G_SEVERITY_WARNING, gtk_info_handler_continue);
> g_info_set_handler (gtk_quark, G_SEVERITY_INFO, gtk_info_handler_continue);
> g_info_set_handler (gtk_quark, G_SEVERITY_DEBUG, gtk_info_handler_continue);

That's what I was trying to avoid. Especially, if there is a chance
we might add new severity levels. I suppose we could define a special
G_SEVERITY_ALL that would, when passed to g_log_set_handler, cause
it to call itself for all severity levels.
 
> but i'd prefer to use g_logger instead of g_info. g_info could be needed
> as a fucntion name later on, and g_logger does express the purpose better.
> it wouldn't for instance be unreasonable if we later feature a comandline
> option, that's going to let the default message handlers log into a
> file or somesuch.

g_logger can't be a function name (or macro name) because it isn't
a verb. g_log() would be OK.
 
> > On GCC, we can #define gtk_warning appropriately on systems without
> > varargs macros, it needs to be a wrapper function (or we can make it
> > an inline function) - in any case, we write:
> > 
> >  gtk_warning ("unable to register error handler")
> > 
> > and even
> >   
> >  gtk_return_if_fail (x != 0);
> > 
> > (The biggest CVS commit ever?)
> 
> hm, i really like the idea of having the same function/macro name for each
> library, so that gtk, gdk, gnome *and* the application all use
> G_WARNING and G_RETURN_IF_FAIL (or g_warning and g_return_if_fail).
> 
> but for the sake of incresed readability i think we should really supply
> lower case variants of those macros, even if that means we need to make
> an exeption to the macros-are-in-upper-case rule.
> we already didn't follow that for the existing g_return_* macros.
 
> e.g. if we have:
> 
> void g_logger (GQuark        who_quark,
>                GSeverityType severity,
>                const gchar  *format,
>                ...);
> 
> #define G_ERROR(fmt, args...) g_logger (G_LOGGER_QUARK, G_SEVERITY_FATAL, fmt, ##args)
> #define G_WARNING(fmt, args...) g_logger (G_LOGGER_QUARK, G_SEVERITY_WARNING, fmt, ##args)
> #define G_MESSAGE(fmt, args...) g_logger (G_LOGGER_QUARK, G_SEVERITY_INFO, fmt, ##args)
> 
> #ifndef G_LOGGER_QUARK
> #define G_LOGGER_QUARK        (0)
> #endif  /* G_LOGGER_QUARK */
> 
> and compile gtk files with -DG_LOGGER_QUARK=gtk_quark, the exception would
> be a few additional defines in glib.h:
> #define	g_warning	G_WARNING
> #define	g_error		G_ERROR
> #define	g_message	G_MESSAGE

I think having both upper and lower case variations is
worse than just upper case. I.e., I  dislike having
lower case macros that do non-functionish things, though I
recognize the aesthetic reasons for such. I don't think
this is a case where we can have our cake and eat it too.
 
> with the sole purpose of increased readability.
> on non-gcc systems, G_WARNING, G_ERROR and G_MESSAGE are even going
> to be *real* functions (inlined), so we could just give them lower
> case names in the first place...

I don't think this is going to work. Remember, we need to pass
an extra argument in and for a varargs function, I think the
only way we can do this on non-GCC systems is:

  g_warning (("%s failed", str));

which presumably fails the huge-commits test. This seems to imply that
everything does need to define it's own warning error and message
functions:-(

But they still should be based on some sort of internal g_log()
thing so everybody doesn't need to define their own handlers
as well.

> extern GQuark gtk_quark; would need to be initialized at the beginning of
> gtk_init(), but do we really want quarks in this place? we can just as well
> export const gchar *gtk_logger_domain = "Gtk", and use
> -DG_LOGGER_DOMAIN=gtk_logger_domain. (this comes pretty close to the usual
> -DPRG_NAME= or -DLIBRARY_NAME= that is used in many projects).

The reason I thought GQuark was to reduce the number of places
that constant strings would appear in each file. I suppose any
decent compiler combines constant strings, so it is only one
extra (short) string for file. That is certainly simpler, and, 
simpler definitely better here. (I'm am still uncertain that
this all is worth the complexity to begin with...)

> basically, g_warning and friends stay the same for the application programmer,
> but for libraries like gtk or gdk, their specific behaviour can be tweaked
> through a -DG_LOGGER_QUARK=something switch, i don't consider that inherently
> evil...

Hmmm, I suppose we could say that it is sort of a function call, that
just happens to provide some more functionality if you defined the
G_LOG_DOMAIN...
 
Regards,
                                        Owen