GNOME Bugzilla – Bug 343012
RC parser rejects lower-case identifiers.
Last modified: 2011-02-04 16:11:04 UTC
When parsing this file: " style "example-style" { gtkmm__CustomObject_mywidget::example_scale = 10 } class "gtkmm__CustomObject_mywidget" style "example-style" " with gtk_parse_rc(), I get this error: custom_gtkrc:3: error: invalid identifier `gtkmm__CustomObject_mywidget', expected valid identifier I don't see the error if I use "Gtkmm__CustomObject_mywidget" instead. As far as I can tell, "gtkmm__CustomObject_mywidget" should be an acceptable identifier. The "_"s don't seem to be the problem either.
Created attachment 66482 [details] testrc.tar.gz Here is a little test case to show the problem.
the GtkRC parser uses the GType string name in order to retrieve the class name. I've never used gtkmm: which name is used to register the GType of gtkmm::CustomObject?
also, the test case is pointless: it fails because there's no registered class called "somewidget", which is true.
> the GtkRC parser uses the GType string name in order to retrieve the class > name. Yes, that's the point. > I've never used gtkmm: which name is used to register the GType of gtkmm::CustomObject? There's no such class. But "Gtkmm__CustomObject_mywidget" is the GType name that would be generated for a gtkmm object that explicitly needs a custom GType name, when supplying "mywidget" as the suffix for that name. > also, the test case is pointless: it fails because there's no registered class called "somewidget", which is true. I'm not convinced that that's why it fails, because it doesn't fail if a a lower-case name is used. But that should be easy to verify in a new test case.
Please keep this bug open until we've at least created that test case and proven your theory. This is obviously a small test case to exercise a problem that we've found in an actual program.
Created attachment 70367 [details] [review] Proposed patch for bug #343012 The parser rejects lower-case character as first character of a namespace/type name. I could not find an obvious reason so I added support for lower-case characters. Maybe this has side-effects, so if someone is familiar with the gtkrc subsystem he should please check this. This patch was made for Openismus <http://www.openismus.com>
The patch breaks something in gtkrc file parsing, at least the themes are not loaded correctly! I have to investigate this further...
> The patch breaks something in gtkrc file parsing, at least the themes are not > loaded correctly! I have to investigate this further... Well, maybe it was just my fault by not correctly installing the rc files inside my jhbuild environment. Seems like the files are parsed correctly.
FWIW, you should probably copy glib/gobject/gtype.c:check_type_name_I to be sure the accepted type names arer the same. I wonder if it'd be useful to make glib export a function to check for valid type names, but that's probably a different bug.
check_type_name_I accepts some more as the parser in gtkrc.c for the first character: gtkrc: A-Za-z (with my patch) check_type_name_I: A-Za-z_ Anyway, I don't know how useful type names starting with '_' are in gtkrc files because I assume this types are private.
Zeah, and who in their right mind would start type names with lower-case letters when everyone uses CamelCase for type names...
OK, so the question is, should I copy check_type_name_I from glib of should it be exported.
Just to be clear, this is not a regression due to parser changes in 2.10, right ?
No, it's not a regression, it's just something that hadn't been tried before with gtkmm.
I wonder if allowing lowercase class names could introduce ambiguities, e.g. if you have classes like "fg" or "base".
Owens comment was: "if you call your class "fg" or "base", you get what you deserve...
So, in the absence of objections, could we commit this? At the least, could we commit this to HEAD and backport it to a stable branch later?
Please suggest some action I can take to move this patch close to being rejected or approved.
Please?
We still need this patch.
The patch author himself said that the patch breaks rc file parsing for some themes; that certainly needs to be investigated before this patch can land.
> The patch author himself said that the patch breaks rc file parsing for some themes If you mean, this comment from Johannes: " The patch breaks something in gtkrc file parsing, at least the themes are not loaded correctly! I have to investigate this further... " He then said " Well, maybe it was just my fault by not correctly installing the rc files inside my jhbuild environment. Seems like the files are parsed correctly. " So there doesn't seem to be any problem. If you still suspect a potential problem, commmiting this now in svn trunk will make sure that it's thoroughly tested by the GNOME 2.19/20 development phase.
> " > Well, maybe it was just my fault by not correctly installing the rc files > inside my jhbuild environment. Seems like the files are parsed correctly. > " Yes, that's what I meant. Even if I didn't apply the patch I did not get the correct theme in jhbuild.
2007-06-10 Matthias Clasen <mclasen@redhat.com> * gtk/gtkrc.c (gtk_rc_parse_style): Accept class names starting with lowercase letters for style property assignments, since GType accepts these too, and gtkmm uses such class names. (#343012, Murray Cumming, Johannes Schmid)