wxrc: XRCWndClassData: minor improvements and `GenerateHeaderCode` bugfix. (PR #24383)

DoctorNoobingstoneIPresume

unread,

Mar 7, 2024, 10:00:22 AM3/7/24

to wx-...@googlegroups.com, Subscribed

For classes with more than one ancestor, the generated C++ code used to end the class definition more than once. This has now been fixed.

Also, for these classes, all constructors now have a default value (nullptr) for the parent parameter.

Other improvements (minor) have been made:

the GetWidgetData member function is now const;
the CanBeUsedWithXRCCTRL member function is now static;
the generated C++ code now uses static_cast instead of C-style cast when preparing the parent argument for the call to InitWidgetsFromXRC;
some string literals are now merged at preprocessing time (as opposed to being added at runtime).

You can view, comment on, or merge this pull request online at:

https://github.com/wxWidgets/wxWidgets/pull/24383

Commit Summary

8263f94 wxrc: XRCWndClassData: minor improvements and `GenerateHeaderCode` bugfix.

File Changes

(1 file)

M utils/wxrc/wxrc.cpp (23)

Patch Links:

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

VZ

unread,

Mar 8, 2024, 1:17:30 PM3/8/24

to wx-...@googlegroups.com, Subscribed

Thanks, I'd just like to understand something: how do you end up with multiple base classes here? I.e. would you have a minimum XRC example reproducing the problem?

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

VZ

unread,

Mar 18, 2024, 9:51:47 AM3/18/24

to wx-...@googlegroups.com, Subscribed

@DoctorNoobingstoneIPresume I wonder if you have seen the question above?

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

DoctorNoobingstoneIPresume

unread,

Mar 20, 2024, 2:47:14 AM3/20/24

to wx-...@googlegroups.com, Subscribed

@DoctorNoobingstoneIPresume I wonder if you have seen the question above?

Thank you for kind replies and please forgive my late reaction...

In wxrc.cpp, I have been looking at the constructor of XRCWndClassData:

    XRCWndClassData(const wxString& className,
                    const wxString& parentClassName,
                    const wxXmlNode* node) :
        m_className(className) , m_parentClassName(parentClassName)
    {
        if ( className == wxT("wxMenu") )
        {
            m_ancestorClassNames.insert(wxT("wxMenu"));
            m_ancestorClassNames.insert(wxT("wxMenuBar"));
        }
        else if ( className == wxT("wxMDIChildFrame") )
        {
            m_ancestorClassNames.insert(wxT("wxMDIParentFrame"));
        }
        else if( className == wxT("wxMenuBar") ||
                    className == wxT("wxStatusBar") ||
                        className == wxT("wxToolBar") )
        {
            m_ancestorClassNames.insert(wxT("wxFrame"));
        }
        else
        {
            m_ancestorClassNames.insert(wxT("wxWindow"));
        }

        BrowseXmlNode(node->GetChildren());
    }

=> The m_ancestorClassNames container has more than one element in the case when the className parameter of the constructor contains the "wxMenu" text.

XRCWndClassData objects are constructred in the PrepareTempFiles function:

...
while (node)
{
    if (node->GetName () == wxT ("object") && node->GetAttribute (wxT ("class"), &classValue) && node->GetAttribute (wxT ("name"), &nameValue))
        aXRCWndClassData.Add (XRCWndClassData (nameValue, classValue, node));
    node = node -> GetNext ();
}
...

If we use the following input XRC file:

<?xml version="1.0" encoding="UTF-8"?>
<resource xmlns="http://www.wxwidgets.org/wxxrc" version="2.5.3.0">
  <object class="This_argument_goes_to_the_parentClassName_parameter" name="wxMenu"></object>
</resource>

and we process it using:

wxrc -c -e -n 'InitXmlResource' -o 'Example3.cpp' 'Example3.xrc'

then the resulting Example3.h header file is:

//
// This file was automatically generated by wxrc, do not edit by hand.
//

#ifndef __Example3_h__
#define __Example3_h__
class wxMenu : public This_argument_goes_to_the_parentClassName_parameter {
protected:

private:
 void InitWidgetsFromXRC(wxWindow *parent){
  wxXmlResource::Get()->LoadObject(this,parent,wxT("wxMenu"), wxT("This_argument_goes_to_the_parentClassName_parameter"));
 }
public:
wxMenu(){
  InitWidgetsFromXRC(nullptr);
 }
};
wxMenu(wxMenu *parent){
  InitWidgetsFromXRC((wxWindow *)parent);
 }
};
wxMenu(wxMenuBar *parent){
  InitWidgetsFromXRC((wxWindow *)parent);
 }
};

void 
'InitXmlResource'();
#endif

and we can see that the class definition for wxMenu is (incorrectly) ended multiple times.

If I have misinterpreted the intention of the code, I wish to ask that you kindly let me know (when there is time).

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

VZ

unread,

Mar 20, 2024, 9:23:08 AM3/20/24

to wx-...@googlegroups.com, Subscribed

(For example, it is not clear to me why users would want to end up with a newly-defined wxMenu class -- a wxMenu class already exists in the wxWidgets project, I think... So why had the original code been prepared for this case ?...)

Well, this is exactly my question, in fact: I don't understand why do we put wxMenu and wxMenuBar into m_ancestorClassNames when the class is wxMenu when it seems like it ought to never be wxMenu in the first place. I.e. if I try to use your example XRC file I still end up with uncompilable header because it redefines the existing wxMenu class.

This code was added back in aac18ec (generate ctors with optional parent parameters in C++ code (patch 1238355), 2006-05-06) and while I have absolutely no memory of this patch and can't find it any more (the page mapping SF ticket numbers to issues is gone now...), I suspect we might want to just revert it instead of trying to fix it if it's useless anyhow.

I've just created #24422 doing this, if you (or anybody else) don't see any problem with it, I'd prefer to apply it and remove this code completely instead of fixing it. Please let me know if you do, thanks!

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

DoctorNoobingstoneIPresume

unread,

Mar 20, 2024, 9:43:24 AM3/20/24

to wx-...@googlegroups.com, Subscribed

Closed #24383.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

DoctorNoobingstoneIPresume

unread,

Mar 20, 2024, 9:43:25 AM3/20/24

to wx-...@googlegroups.com, Subscribed

I certainly agree with you, and I will close this pull request if it is alright with you, and thank you for patience, that you have given me time so I can learn myself.

(Indeed, I too was surprized, that the name of the object, not the name of the class of the object, had to be wxMenu in order for that path to be taken...)

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

paulcor

unread,

Mar 20, 2024, 10:54:37 AM3/20/24

to wx-...@googlegroups.com, Subscribed

and while I have absolutely no memory of this patch and can't find it any more

#7433

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Reply all

Reply to author

Forward