On Mon, May 31, 2010 at 08:49:51PM -0700, Alex Leone wrote:
I've been working on the document properties dialog, which uses RegisteredWidgets (essentially wrappers that link a gtk widget to an XML attribute, so changing the widget changes the XML), and I've noticed that the code doesn't really use the attribute constants in attributes.h.
(Note that I answer without having looked at the above-referenced code.)
My question: instead of using straight strings, for example "inkscape:document-units", should the code be using
sp_attribute_name(SP_ATTR_INKSCAPE_DOCUMENT_UNITS)
There could be some value in having a macro for each attribute (defined as a string literal) so that typos are caught at compile time, but I wouldn't go so far as to add a function call at each such place.
However, some advantages of retaining string literals instead of macros are that they're a bit more readable and easier to grep for.
Some other possible approaches to checking:
- A macro defined as:
#define ATTR(_lit) (static_assert(_lit == "id" \ || _lit == "inkscape:collect" \ ... || _lit == "requiredExtensions" \ || _lit == "effect"), \ _lit)
However, it still has some readability cost, and it slows down each compile.
- A script to scan Inkscape for all string literals that aren't arguments to gettext or g_log, and put it in a sorted list. Create a whitelist from http://www.w3.org/TR/SVG11/attindex.html and eltindex.html and propidx.html and the inkscape/sodipodi namespaced entries in attributes.cpp, and filter those from the sorted list (say using comm). Manually check the remaining strings once, then add them to the whitelist. Then have a check (possibly part of `make check') that runs this script and warns if there are any new string literals not in those whitelists: these are potential typos.
The script sounds fun to write, but I think one would only write it for fun rather than need: I don't recall us having bugs of typos in attributes or element names.
So, coming back to your actual question, I think the answer is "no, string literals should be fine."
pjrm.