GNOME Bugzilla – Bug 668924
Make gedit_debug_message() introspectable
Last modified: 2012-02-09 20:10:13 UTC
With gedit_debug_message() part of the public API (http://developer.gnome.org/gedit/3.0/gedit-gedit-debug.html#gedit-debug-message), it should be introspectable so that plugins can call it. The GIR scanner generates: <function name="debug_message" c:identifier="gedit_debug_message" introspectable="0"> <return-value transfer-ownership="none"> <type name="none" c:type="void"/> </return-value> <parameters> <parameter name="section" transfer-ownership="none"> <type name="DebugSection" c:type="GeditDebugSection"/> </parameter> <parameter name="file" transfer-ownership="none"> <type name="utf8" c:type="gchar*"/> </parameter> <parameter name="line" transfer-ownership="none"> <type name="gint" c:type="gint"/> </parameter> <parameter name="function" transfer-ownership="none"> <type name="utf8" c:type="gchar*"/> </parameter> <parameter name="format" transfer-ownership="none"> <type name="utf8" c:type="gchar*"/> </parameter> <parameter transfer-ownership="none"> <varargs> </varargs> </parameter> </parameters> </function> but `introspectable="0"` makes it inaccessible to plugins. The problem is apparently that gobject-introspection does not support variadic functions by default: http://live.gnome.org/Vala/UpstreamGuide
Created attachment 206754 [details] [review] Fix (Adds two functions to the gedit-debug public API)
BTW, I am testing the patch with my Line Ending Style plugin <https://github.com/dtrebbien/gedit-line-ending-style-plugin>. Running Gedit compiled with the patch (and GEDIT_DEBUG_PLUGINS=1), the latest version of the plugin logs messages such as: [8.704091 (8.130193)] /home/daniel/.local/share/gedit/plugins/lineendingstyle.py:453 (LineEndingStylePlugin.__init__) self=<LineEndingStylePlugin object at 0x2d130a0 (LineEndingStylePlugin at 0x292e320)> [8.704292 (0.000201)] /home/daniel/.local/share/gedit/plugins/lineendingstyle.py:456 (LineEndingStylePlugin.__del__) self=<LineEndingStylePlugin object at 0x2d130a0 (LineEndingStylePlugin at 0x292e320)> [8.708426 (0.004134)] /home/daniel/.local/share/gedit/plugins/lineendingstyle.py:312 (LineEndingStylePluginUI.__init__) self=<lineendingstyle.LineEndingStylePluginUI instance at 0x2efbab8>, window=<Window object at 0x2cd1e10 (GeditWindow at 0x21dc020)> [8.713646 (0.005220)] /home/daniel/.local/share/gedit/plugins/lineendingstyle.py:89 (GeditStatusComboBox.__init__) self=<GeditStatusComboBox object at 0x2f7c730 (lineendingstyle+GeditStatusComboBox at 0x2422600)> Running without the patch, the plugin logs only the trace information. E.g.: [10.685079 (10.092980)] /home/daniel/.local/share/gedit/plugins/lineendingstyle.py:453 (LineEndingStylePlugin.__init__) [10.686092 (0.001013)] /home/daniel/.local/share/gedit/plugins/lineendingstyle.py:456 (LineEndingStylePlugin.__del__) [10.688836 (0.002744)] /home/daniel/.local/share/gedit/plugins/lineendingstyle.py:312 (LineEndingStylePluginUI.__init__) [10.693941 (0.005105)] /home/daniel/.local/share/gedit/plugins/lineendingstyle.py:89 (GeditStatusComboBox.__init__)
Hi Daniel, first of all thanks for the patch. I am not sure this is the direction we want to take though... If we really want to add a variant of the function to be used through introspection I think it should probably be a gedit_debug_messagev that takes a va_list so that bindings can bind their own message formatting stuff. I have not looked in detail but there are many other cases in gtk where vararg functions have a "v" variat for the sake of bindings so I hope introspection can handle that. On a more general note I am not sure we want to push plugins to use this api though, glib logging support has been recently improved and we are considering to drop gedit_debug completely.
Created attachment 206865 [details] [review] Fix (Adds three functions to the gedit-debug public API) Some changes per discussion on #gedit.
In the mean time I pushed the first 3 cleanup/docs patches
Created attachment 206936 [details] [review] Fix (Adds three functions to the gedit-debug public API) Updates the "Since: " lines now that Gedit 3.3.3 has been released.
Created attachment 206989 [details] [review] Patch to relocate GTK-Doc stuff to the correct place Per discussion on #gedit, this patch moves the gedit-debug GTK-Doc blocks from gedit-debug.h to gedit-debug.c.
Created attachment 206990 [details] [review] Fix (Adds two functions to the gedit-debug public API) Per discussion on #gedit yesterday, this patch adds a gedit_debug_plugin_message() function.
Review of attachment 206990 [details] [review]: ::: gedit/Gedit.py @@ -43,2 +45,5 @@ __all__.append('Message') + + +def get_filename(num_back_frames=0): What about merging these three in a single function returning a tuple, since we use them together. It would also mean calling currentframe() etc one third of the times and probably shorten the code ::: gedit/gedit-debug.c @@ -58,2 +59,3 @@ enum GeditDebugSection; +/* Holds the current set of debug section flags, output for which has been "output for which" sounds a bit awkward in english... to be honest I'd just skip the comment, public interfaces should be well documented but internal code should be clear enough to not require too many comments in the first place :) @@ -60,2 +63,3 @@ static GeditDebugSection debug = GEDIT_NO_DEBUG; +/* Expands to a boolean expression for whether #GeditDebugSection @section_rval once again no need to overdocument things, it is quite clear what this does and it is an internal detail @@ +151,3 @@ const gchar *function) { + va_list dummy_format_args; this seems to pass dummy_format_args uninitialized... I know that we are not going to use it, but seems strange that the compiler does not complain. Anyway as I said before I would not bother with the _valist function ::: gedit/gedit-debug.h @@ -92,0 +96,6 @@ +void gedit_debug_plugin_message (const gchar *file, + gint line, + const gchar *function, ... 3 more ... Given the results of our investigations in g-i I would not add this variant: it is just one more extra symbol exported that nobody uses and the amount of code shared is actually outweighted by having yet anoter function etc
Created attachment 207038 [details] [review] Fix (Adds gedit_debug_plugin_message() to the gedit-debug public API)
Review of attachment 207038 [details] [review]: ::: gedit/gedit-debug.c @@ -188,0 +249,2 @@ +void +debug_message_valist (GeditDebugSection section, What I meant in the previous review is that this function is not useful and should be removed. Basically the patch should just add the new _plugin_message function without all the rest of the refactoring ::: gedit/gedit-debug.h @@ -36,1 +36,2 @@ #include <glib.h> +#include <stdarg.h> not needed anymore
Created attachment 207199 [details] [review] Fix (Adds gedit_debug_plugin_message() to the gedit-debug public API)
Review of attachment 207199 [details] [review]: Ok, finally committed this, thanks for your patience :) I actually made a couple of small changed before pushing: - I removed /*< private *>/ etc from the enum because glib-mkenums was giving an error - I restored the fact that the format argument is mandatory for debug_message since not only there is no good reason for passing null, but also because it is marked with G_GNUC_PRINTF