2.99 Enhance ScriptFu error handling and debug, patch attached
To fix these issues:
-
omits domain on g_error_new_literal which throws GLib-WARNING **: 11:29:10.711: (../../../glib/gerror.c:412):g_error_new_valist: runtime check failed: (domain != 0)
-
Many script error messages do not provide useful details. For example, "Invalid type for argument" omits to says what formal Lisp type was expected (e.g. vector, list, number, string)
-
messages do not distinguish errors by script authors from errors by the ScriptFu developers.
-
Debugging code is implemented by #ifdef spread throughout the code making it hard to read.
-
Debugging code has not been maintained and does not compile.
-
There is no test suite
-
A few other small bugs (e.g. missing break on one switch case)
About the patch
For reviewers of the patch.
TLDR: makes the code easier to read/change, and reduces size of scheme-wrapper.c by hundreds of lines. No change in behaviour, except for changes in error message texts or debug log.
Refactors by extracting functions for declaring errors in scripts. The functions/errors are of two kinds:
- plugin author's errors in scripts
- implementation errors in ScriptFu or in called PDB procedures.
Functions are extracted to script-fu-errors.c
Refactors by extracting functions for debugging (dumping large structs) Functions are extracted to script-fu-errors.c (same as functions for errors)
Uses GLib logging (g_debug convenience function) instead of g_printerr and #ifdef. Thus debugging ScriptFu is now:
- controllable at runtime by defining G_MESSAGE_DEBUG=scriptfu
- log messages follow conventions and print the domain "scriptfu"
In my opinion, there is no need to worry about performance in ScriptFu. However, in the future one could easily:
- conditionally compile the debug functions that were extracted.
- additionally wrap all the g_debug() calls remaining in scheme-wrapper.c into a conditionally compiled wrapper function
Related issues
I suggest applying this patch first. Then other needed patches will be easier.
Some discussion of the future of ScriptFu re GI in #5402 (closed), #5426 (closed)
Other scriptfu issues:
- #5402 (closed) (ScriptFu unhandled GIMP types)
- #5426 (closed) (need compatibility for changed PDB API re ID vs object)
- #6026 (closed) (need compatibility for changed PDB API re multilayer)
Testing
I tested thoroughly (if not quite exhaustively.) I did not test exhaustively because it is more important to fix other issues in ScriptFu that are extant in GIMP repository scripts (which will add test cases) than to test cases that are extremely rare, e.g. in arg is STRING_ARRAY. Since there is still some question about the future of ScriptFu re GI, no point in testing something that might be scrapped.
A test plugin is at https://github.com/bootchk/testGimpScriptFuBinding.git It is a GimpFu Python plugin that calls plugin-script-fu-eval on Scheme constructs, once per test case. Since is Python, easy to add test cases.