After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 343012 - RC parser rejects lower-case identifiers.
RC parser rejects lower-case identifiers.
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
2.8.x
Other Linux
: Normal normal
: ---
Assigned To: Johannes Schmid
gtk-bugs
Depends on:
Blocks: 396198
 
 
Reported: 2006-05-26 11:17 UTC by Murray Cumming
Modified: 2011-02-04 16:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
testrc.tar.gz (468 bytes, application/x-gzip)
2006-05-30 17:30 UTC, Murray Cumming
  Details
Proposed patch for bug #343012 (1.43 KB, patch)
2006-08-07 10:28 UTC, Johannes Schmid
none Details | Review

Description Murray Cumming 2006-05-26 11:17:57 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.
Comment 1 Murray Cumming 2006-05-30 17:30:05 UTC
Created attachment 66482 [details]
testrc.tar.gz

Here is a little test case to show the problem.
Comment 2 Emmanuele Bassi (:ebassi) 2006-08-04 16:31:03 UTC
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?
Comment 3 Emmanuele Bassi (:ebassi) 2006-08-04 16:38:51 UTC
also, the test case is pointless: it fails because there's no registered class called "somewidget", which is true.
Comment 4 Murray Cumming 2006-08-05 13:39:19 UTC
> 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.
Comment 5 Murray Cumming 2006-08-05 13:40:40 UTC
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.
Comment 6 Johannes Schmid 2006-08-07 10:28:16 UTC
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>
Comment 7 Johannes Schmid 2006-08-07 11:54:48 UTC
The patch breaks something in gtkrc file parsing, at least the themes are not loaded correctly! I have to investigate this further...
Comment 8 Johannes Schmid 2006-08-07 12:05:06 UTC
> 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.

Comment 9 Benjamin Otte (Company) 2006-08-08 16:17:35 UTC
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.
Comment 10 Johannes Schmid 2006-08-09 11:03:21 UTC
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.
Comment 11 Benjamin Otte (Company) 2006-08-10 14:20:52 UTC
Zeah, and who in their right mind would start type names with lower-case letters when everyone uses CamelCase for type names...
Comment 12 Johannes Schmid 2006-08-11 12:06:03 UTC
OK, so the question is, should I copy check_type_name_I from glib of should it be exported.
Comment 13 Matthias Clasen 2006-08-14 19:20:52 UTC
Just to be clear, this is not a regression due to parser changes in 2.10, right ?
Comment 14 Murray Cumming 2006-08-14 20:26:55 UTC
No, it's not a regression, it's just something that hadn't been tried before with gtkmm.
Comment 15 Matthias Clasen 2006-09-28 15:20:24 UTC
I wonder if allowing lowercase class names could introduce ambiguities, e.g.
if you have classes like "fg" or "base".
Comment 16 Matthias Clasen 2006-09-28 15:37:26 UTC
Owens comment was: "if you call your class "fg" or "base", you get what you deserve...

Comment 17 Murray Cumming 2006-10-18 13:09:11 UTC
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?
Comment 18 Murray Cumming 2006-11-17 08:15:41 UTC
Please suggest some action I can take to move this patch close to being rejected or approved.
Comment 19 Murray Cumming 2007-01-16 15:21:02 UTC
Please?
Comment 20 Murray Cumming 2007-05-07 06:01:00 UTC
We still need this patch.
Comment 21 Murray Cumming 2007-06-10 09:27:44 UTC
Please?
Comment 22 Matthias Clasen 2007-06-10 15:48:29 UTC
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.
Comment 23 Murray Cumming 2007-06-10 16:03:21 UTC
> 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. 

Comment 24 Johannes Schmid 2007-06-10 16:59:45 UTC
> "
> 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.
Comment 25 Matthias Clasen 2007-06-11 04:45:16 UTC
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)