GNOME Bugzilla – Bug 708212
g_variant_parser_error_get_quark() has unexpected name.
Last modified: 2013-12-22 16:28:58 UTC
Most GErrors, such as GSomethingError, have a function to get their quark that looks like g_something_error_quark(), so bindings (such as gtkmm) would expect GVariantParseError to have g_variant_parse_error_quark(). Instead it has g_variant_parser_get_error_quark(). This patch deprecates the old function and adds the correct one, making life easier for gtkmm (and maybe others).
Created attachment 255078 [details] [review] 0001-GVariant-Add-g_variant_parse_error_quark.patch
What's wrong with G_VARIANT_PARSE_ERROR?
We're not guaranteeing the names of these functions. I think it would be much preferable to tweak the heuristics you are using to generate your binding than to add redundant symbols to glib
we actually have been saying that you ought to write error quark functions as <namespace>_<module>_error_quark() for a *long* while: https://wiki.gnome.org/GObjectIntrospection/WritingBindingableAPIs
Ugh. This recommendation flies in the face of the convention that functions should not be named after nouns...
just so it's not a G-I issue on a wiki, the official documentation for GError specifies the convention: https://developer.gnome.org/glib/stable/glib-Error-Reporting.html """ Error domains and codes are conventionally named as follows: The error domain is called <NAMESPACE>_<MODULE>_ERROR, for example G_SPAWN_ERROR or G_THREAD_ERROR: The quark function for the error domain is called <namespace>_<module>_error_quark, for example g_spawn_error_quark() or g_thread_error_quark(). The error codes are in an enumeration called <Namespace><Module>Error; for example,GThreadError or GSpawnError. Members of the error code enumeration are called <NAMESPACE>_<MODULE>_ERROR_<CODE>, for example G_SPAWN_ERROR_FORK or G_THREAD_ERROR_AGAIN. If there's a "generic" or "unknown" error code for unrecoverable errors it doesn't make sense to distinguish with specific codes, it should be called <NAMESPACE>_<MODULE>_ERROR_FAILED, for example G_SPAWN_ERROR_FAILED. """ we've been following this convention throughout GNOME and GLib-based API for a *very* long time; all our error domain quark functions follow it. (In reply to comment #5) > Ugh. This recommendation flies in the face of the convention that functions > should not be named after nouns... which is a recommendation we never really had encoded anywhere, as it's the first time I heard about it. anyway, error domain quark functions are pretty much an implementation detail of how error domains work. for consistency with the rest of the platform, I'd commit this patch.
Would be nice to have some annotation that lets us tell g-i which enum and quark belong together.
Review of attachment 255078 [details] [review]: I'm convinced. After the release, though, please. ::: glib/gvariant.h @@ +329,3 @@ GQuark g_variant_parser_get_error_quark (void); GLIB_AVAILABLE_IN_ALL This is tricky. For people who use the function directly, this is wrong. It should be GLIB_AVAILABLE_IN_2_40. For people who use the function via the macro, this is correct because otherwise we could get spurious warnings for people who are just blindly using the macro. But that's sort of an interesting point: if people are building against a newer version of GLib, but have their version depends pinned back to an old version, do we expect to generate a binary that is ABI-compatible with the old version? We have never really explicitly talked about this in context of version macros... which you apparently define to ensure you're maintaining compat with older version. Probably not an issue on Linux since we're coding the soname into the binary, but vaguely interesting on Windows where we just have an unversioned .dll... Maybe worth mentioning in the docs for the version macros: "No fowards-compatible ABI guarantees. Build against the old version".
(In reply to comment #8) > But that's sort of an interesting point: if people are building against a newer > version of GLib, but have their version depends pinned back to an old version, > do we expect to generate a binary that is ABI-compatible with the old version? > We have never really explicitly talked about this in context of version > macros... we have actually. The expansion of G_DEFINE_TYPE (or more specifically, _G_DEFINE_TYPE_EXTENDED_CLASS_INIT) is different depending on "#if GLIB_VERSION_MAX_ALLOWED >= GLIB_VERSION_2_38"
So maybe we should try a little bit harder here as well? Do we seriously expect to be ABI-forwards-compatible though, assuming correct usage of version macros? That still seems like quite a tall order...
Let's commit this already. I honestly don't think the fowards-backwards-compatible-ABI thing is important in light of the fact that we're surely violating this in other ways already. If anyone else wants to come forward with a patch then OK, but otherwise, this is fixed.