Patch to issue warning when attempting to set or get a non-existent preference
Hi all,
Currently when you try to access the value of a preference setting within Inkscape, no warning is issued if the requested preference doesn't exist. Instead, a default value provided by the caller is returned.
This can cause bugs if there is a typo in the preference name, or if the pref is not created properly in src/preferences-skeleton.h. The latter problem caused the 'oversample bitmaps' preference to be broken for at least 2 releases (0.44 and 0.45) (bug 167824 and bug 168346).
I wrote a patch to issue a warning when the requested preference doesn't exist, and found that this 'silent failure' behavior is actually used in a few places to allow debugging parameters to be passed in by manually adding items to your preferences.xml file. I saw warnings for 'options.filtertest', 'options.bulia', and 'debug.icons', but there may be more in code paths that I just haven't passed through yet.
I can see 3 ways to address this usage while adding the new warning. I have implemented #3 in the attached patch.
1) Deprecate/disallow the use of the preferences file for passing in arbitrary debugging parameters. pro: "clean" con: would remove useful functionality; hard to systematically find all the debugging pref names
2) Introduce a flag in the preferences get/set functions that suppresses the warning. pros: debugging usage would be explicit; no changes to current prefs names con: would have to rewrite all callers
3) Do not issue a warning for prefs that begin with "debug.", and move existing debug prefs to this namespace (options.filtertest becomes debug.filtertest, etc.) pro: captures the developer's intent to use the pref for debugging, without having to rewrite function calls as in #2 con: developers who currently use these flags would have to manually change their preferences.xml files
I chose to implement the warning in inkscape_get_repr in src/inkscape.cpp, rather than in the helper functions in src/prefs-utils.cpp, in order to avoid duplication. This may be a bit late for 0.46? Since it's just adding a warning, it seems like a low risk patch, but I don't have SVN access in any case, so will defer to the wisdom of the list...
Thoughts/suggestions welcome: I'm new to this! -Tom
On Feb 20, 2008 4:01 AM, Tom Davidson <tjd@...1007...> wrote:
'options.bulia'
that one wasn't mine, it was jon cruz doing something at my request :)
- Deprecate/disallow the use of the preferences file for passing in
arbitrary debugging parameters. pro: "clean" con: would remove useful functionality; hard to systematically find all the debugging pref names
I think this is the way to go. Debugging should be ifdefed out or removed. Debugging which is itself flakey and may not work because of missing element in the XML file is worse than no debugging.
- Do not issue a warning for prefs that begin with "debug.", and move
existing debug prefs to this namespace (options.filtertest becomes debug.filtertest, etc.)
This is the second preference, if someone insists on keeping them, although I really don't think we need these debug prefs in public SVN - either use and keep them in your tree while debugging then throw them away OR properly document, rename to something sensible, and add to preferences-skeleton.
Wed, 20 Feb 2008 00:01:32 -0800 "Tom Davidson" <tjd@...1007...> kirjoitti:
I saw warnings for 'options.filtertest', 'options.bulia', and 'debug.icons', but there may be more in code paths that I just haven't passed through yet.
Wait, what? Something in Inkscape codebase still tries to access options.filtertest? Now that we have nice GUI for filters and everything, that's highly obsolete.
participants (3)
-
bulia byak -
Niko Kiirala -
Tom Davidson