Author Topic: new wxWidget(wxSmith) project  (Read 22605 times)

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5493
new wxWidget(wxSmith) project
« on: June 21, 2007, 10:35:54 pm »
I just created a new wxWidgets project and choose wxSmith as the gui builder. I choose the dialog based app.
On linux !
Unfortunately it doesn't compile, probably doesn't know where the headers are, seems this is not dealed with by the wizard ??
[Edit] : fixed : I can build now my build options needed to be changed to `wx-config-2.8 --cflags` --> append of -2.8

But I have some important remarks with the generated code, could one of the wizards gurus adopt it please.

In total 5 files are generated (I called my project wxSmith1) :

1) wxSmith1App.h
this header files is including precompiled headers, one should not to do in header files !!
So I would suggest to remove the following construct :
Code
#include <wx/wxprec.h>

#ifdef __BORLANDC__
    #pragma hdrstop
#endif

#ifndef WX_PRECOMP
    #include <wx/wx.h>
#endif

by just :
Code
#include <wx/app.h>

Remark : I never got the choice to say in I want pch or not !!!
Question : is that _BORLANDC_ pragma really needed. I just looks and feels plain ugly ?

2) wxSmith1App.cpp
- again that _BORLANDC_ stuuf is there --> needed (note : maybe here, but for sure not in the header file ?)
- do NOT use NULL, that's not standard C/C++ ==> the correct value is : 0


3) wxSmith1Main.h
This is something where wxSmith also is not doing the best it could do: In the header file it is including a bunch of headers which are not needed at that point, forward declaration are sufficient. Those headers are only needed at the cpp.
Could this please be changed, so there are less dependencies in the headers.

So in my example this :
Code
#include "wxSmith1App.h"

//(*Headers(wxSmith1Dialog)
#include <wx/button.h>
#include <wx/dialog.h>
#include <wx/sizer.h>
#include <wx/statline.h>
#include <wx/stattext.h>
//*)

class wxSmith1Dialog: public wxDialog
{
public:

wxSmith1Dialog(wxWindow* parent,wxWindowID id = -1);
virtual ~wxSmith1Dialog();

private:

//(*Handlers(wxSmith1Dialog)
void OnQuit(wxCommandEvent& event);
void OnAbout(wxCommandEvent& event);
//*)

//(*Identifiers(wxSmith1Dialog)
static const long ID_STATICTEXT1;
static const long ID_BUTTON1;
static const long ID_STATICLINE1;
static const long ID_BUTTON2;
//*)

//(*Declarations(wxSmith1Dialog)
wxBoxSizer* BoxSizer1;
wxStaticText* StaticText1;
wxBoxSizer* BoxSizer2;
wxButton* Button1;
wxStaticLine* StaticLine1;
wxButton* Button2;
//*)

DECLARE_EVENT_TABLE()
};

could, well actually should become :

Code
#include "wxSmith1App.h"

//(*Headers(wxSmith1Dialog)
#include <wx/dialog.h>
class wxButton;
class wxBoxSizer;
class wxStaticText;
class wxStaticLine;
class wxCommandEvent;
//*)

class wxSmith1Dialog: public wxDialog
{
public:

wxSmith1Dialog(wxWindow* parent,wxWindowID id = -1);
virtual ~wxSmith1Dialog();

private:

//(*Handlers(wxSmith1Dialog)
void OnQuit(wxCommandEvent& event);
void OnAbout(wxCommandEvent& event);
//*)

//(*Identifiers(wxSmith1Dialog)
static const long ID_STATICTEXT1;
static const long ID_BUTTON1;
static const long ID_STATICLINE1;
static const long ID_BUTTON2;
//*)

//(*Declarations(wxSmith1Dialog)
wxBoxSizer* BoxSizer1;
wxStaticText* StaticText1;
wxBoxSizer* BoxSizer2;
wxButton* Button1;
wxStaticLine* StaticLine1;
wxButton* Button2;
//*)

DECLARE_EVENT_TABLE()
};

4) wxSmith1Main.cpp
Let's first show the code and then discuss what should be changed :

Code
#ifdef WX_PRECOMP
#include "wx_pch.h"
#endif

#ifdef __BORLANDC__
#pragma hdrstop
#endif //__BORLANDC__

#include "wxSmith1Main.h"

//helper functions
enum wxbuildinfoformat {
    short_f, long_f };

wxString wxbuildinfo(wxbuildinfoformat format)
{
    wxString wxbuild(wxVERSION_STRING);

    if (format == long_f )
    {
#if defined(__WXMSW__)
        wxbuild << _T("-Windows");
#elif defined(__UNIX__)
        wxbuild << _T("-Linux");
#endif

#if wxUSE_UNICODE
        wxbuild << _T("-Unicode build");
#else
        wxbuild << _T("-ANSI build");
#endif // wxUSE_UNICODE
    }

    return wxbuild;
}

//(*InternalHeaders(wxSmith1Dialog)
#include <wx/bitmap.h>
#include <wx/font.h>
#include <wx/fontenum.h>
#include <wx/fontmap.h>
#include <wx/image.h>
#include <wx/intl.h>
#include <wx/settings.h>
//*)

//(*IdInit(wxSmith1Dialog)
const long wxSmith1Dialog::ID_STATICTEXT1 = wxNewId();
const long wxSmith1Dialog::ID_BUTTON1 = wxNewId();
const long wxSmith1Dialog::ID_STATICLINE1 = wxNewId();
const long wxSmith1Dialog::ID_BUTTON2 = wxNewId();
//*)

BEGIN_EVENT_TABLE(wxSmith1Dialog,wxDialog)
//(*EventTable(wxSmith1Dialog)
//*)
END_EVENT_TABLE()

wxSmith1Dialog::wxSmith1Dialog(wxWindow* parent,wxWindowID id)
{
//(*Initialize(wxSmith1Dialog)
Create(parent,id,_("wxWidgets app"),wxDefaultPosition,wxDefaultSize,wxDEFAULT_DIALOG_STYLE,_T("wxDialog"));
BoxSizer1 = new wxBoxSizer(wxHORIZONTAL);
StaticText1 = new wxStaticText(this,ID_STATICTEXT1,_("Welcome to\nwxWidgets"),wxDefaultPosition,wxDefaultSize,0,_T("ID_STATICTEXT1"));
wxFont StaticText1Font = wxSystemSettings::GetFont(wxSYS_DEFAULT_GUI_FONT);
if ( !StaticText1Font.Ok() ) StaticText1Font = wxSystemSettings::GetFont(wxSYS_DEFAULT_GUI_FONT);
StaticText1Font.SetPointSize(20);
StaticText1->SetFont(StaticText1Font);
BoxSizer1->Add(StaticText1,1,wxALL|wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL,10);
BoxSizer2 = new wxBoxSizer(wxVERTICAL);
Button1 = new wxButton(this,ID_BUTTON1,_("About"),wxDefaultPosition,wxDefaultSize,0,wxDefaultValidator,_T("ID_BUTTON1"));
BoxSizer2->Add(Button1,1,wxALL|wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL,4);
StaticLine1 = new wxStaticLine(this,ID_STATICLINE1,wxDefaultPosition,wxSize(10,-1),wxLI_HORIZONTAL,_T("ID_STATICLINE1"));
BoxSizer2->Add(StaticLine1,0,wxALL|wxEXPAND|wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL,4);
Button2 = new wxButton(this,ID_BUTTON2,_("Quit"),wxDefaultPosition,wxDefaultSize,0,wxDefaultValidator,_T("ID_BUTTON2"));
BoxSizer2->Add(Button2,1,wxALL|wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL,4);
BoxSizer1->Add(BoxSizer2,0,wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL,4);
SetSizer(BoxSizer1);
BoxSizer1->Fit(this);
BoxSizer1->SetSizeHints(this);
Connect(ID_BUTTON1,wxEVT_COMMAND_BUTTON_CLICKED,(wxObjectEventFunction)&wxSmith1Dialog::OnAbout);
Connect(ID_BUTTON2,wxEVT_COMMAND_BUTTON_CLICKED,(wxObjectEventFunction)&wxSmith1Dialog::OnQuit);
//*)
}

wxSmith1Dialog::~wxSmith1Dialog()
{
//(*Destroy(wxSmith1Dialog)
//*)
}

void wxSmith1Dialog::OnQuit(wxCommandEvent& event)
{
    Close();
}

void wxSmith1Dialog::OnAbout(wxCommandEvent& event)
{
    wxString msg = wxbuildinfo(long_f);
    wxMessageBox(msg, _("Welcome to..."));
}

- first of all all headers should be included at the top, not first some code and then some headers. In this case this can lead to compile errors, since when there's no pch -> wx_pch.h not included. And we already encounter some code for using undefined types at that moment : wxString, _T(), and only after these first implementation lines of code we see a next bunch of includes. Finally bringing in wx/intl.h (_T()), but still no wxString. So in this example not all needed headers are included !!!
Short word : this code is wrong.
- so the second bunch of headers should move up, and the headers I removed in 3) should join them in the cpp file, and the construct should be something like this :

#ifdef WX_PRECOMP
#include "wx_pch.h"
#else
 // include here all headers you need and that in the pch case would be brought in by the wx_pch.h (and hope this never changes in the future (extension is ok, but reduction not)
#endif
// include here all other wx headers needed but that are not delivered by wx_pch.h [so they get included always: pch and non-pch]

- I didn't check the rest of the code, but 1 thing I already noticed was no header included for wxMessageBox, so the include of <wx/utils.h> should be added otherwise wcode

5) wxsmith1dialog.wxs
As said, the name of my project was wxSmith1, all previous 4 generated files are CamelCase, only this one is NOT.
For consistency, it would be best it also follow the CamelCase convention ==> wxSmith1Dialog.wxs


Once again I would like to stress : please use headers and there includes correctly. You get less dependencies and better portable code [most of the time for example if the linux build is broken in CB it is due to incorrect header inclusions]. So this is extremely important.

Aside of that : wxSmith rules !!!


EDIT : I would suggest to get rid of that _BORLANDC_ thing, since CB code is purely targeted for GCC. And I really wonder if Borland still needs such an ugly beast.
« Last Edit: June 23, 2007, 10:38:39 am by killerbot »

Offline byo

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 837
Re: new wxWidget(wxSmith) project
« Reply #1 on: June 21, 2007, 11:51:22 pm »
Ok, I'll take all those things into consideration :) Very good report indeed.

Remark : I never got the choice to say in I want pch or not !!!
Question : is that _BORLANDC_ pragma really needed. I just looks and feels plain ugly ?

I don't like it either, but don't know what will be the result on borland compiler. In the worst case it will just slow down the compilation. This code is just taken from wx examples so it's the suggested way. But I also would like to get rid of it.

Quote
- do NOT use NULL, that's not standard C/C++ ==> the correct value is : 0

Right, it should be easy to find and replace.

Quote
This is something where wxSmith also is not doing the best it could do: I nthe header file it is including a bunch of headers which are not needed at that point, forward declaration are sufficient. Those headers are only needed at the cpp.

I'll have to consider what code changes it will require. It may introduce some problems when someone would like to write inline function accessing controls. So I'll probably leave this problem for later.

Quote
- first of all all headers should be included at the top, not first some code and then some headers.

True, true...

Quote
Finally bringing in wx/intl.h (_T()), but still no wxString. So in this example not all needed headers are included !!!

Actually <wx/intl.h> internally includes <wx/string.h>. And it's certain that at least any other include will also use <wx/string.h> (what can not be said about <wx/intl.h>). But you may be right that this "list" doesn't look complete ;) and including <wx/string.h> shouldn't be time-consuming. So I'll probably add it.

Quote
#ifdef WX_PRECOMP
#include "wx_pch.h"
#else
 // include here all headers you need and that in the pch case would be brought in by the wx_pch.h (and hope this never changes in the future (extension is ok, but reduction not)
#endif
// include here all other wx headers needed but that are not delivered by wx_pch.h [so they get included always: pch and non-pch]

Huh, that may be challenging since wxSmith doesn't split headers into pch-included and the rest (I just wonder why there are such headers that are not in pch). Since it may require changes of bigger wxSmith code parts, I'll leave it as post-1.0 task. Currently I'll just threat all headers as non-pch. They sohuldn't slow things down very much since header guard macros simply elliminate everything at preprocessor level.

Quote
- I didn't check the rest of the code, but 1 thing I already noticed was no header included for wxMessageBox, so the include of <wx/utils.h> should be added otherwise wcode

Actually, here it would be <wx/msgdlg.h> ;)

Quote
As said, the name of my project was wxSmith1, all previous 4 generated files are CamelCase, only this one is NOT.
Not a problem. It followed previous scheme used for class wizard (all file names lower case).

Quote
Once again I would like to stress : please use headers and there includes correctly. You get less dependencies and better portable code [most of the tie for example if the linux build is broken in CB it is due to incorrect header inclusions]. So this is extremely important.

Hmm, I agree only partially. Of course I agree that headers should (in most cases) reside at the beginning of file. But forward declaration may cause more problems than few extre includes and for sure it wont do any harm when including external library (forward declarations are required only when there's case of cyclic includes), just a slower compilation.

Quote
Aside of that : wxSmith rules !!!

 8) Glad to hear that :)

Regards
  BYO

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5493
Re: new wxWidget(wxSmith) project
« Reply #2 on: June 22, 2007, 12:04:14 am »
Quote

Quote
- I didn't check the rest of the code, but 1 thing I already noticed was no header included for wxMessageBox, so the include of <wx/utils.h> should be added otherwise wcode

Actually, here it would be <wx/msgdlg.h> Wink

Correct, my mistake.


EDIT : normally forward declarations won't bring any problems, since for the compiler it's more then enough info, and as such if the full header include would have been there, you create a dependency on those files, if they change everyone using your header also recompiles, where only your implementation should recompile. The user of your hader doesn't care that internally there's a wxButton, but for your implementation this is very important ;-) .

« Last Edit: June 22, 2007, 12:09:59 am by killerbot »

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: new wxWidget(wxSmith) project
« Reply #3 on: June 23, 2007, 06:08:53 am »
I just created a new wxWidgets project and choose wxSmith as the gui builder. I choose the dialog based app.
On linux !
Unfortunately it doesn't compile, probably doesn't know where the headers are, seems this is not dealed with by the wizard ??
[Edit] : fixed : I can build now my build options needed to be changed to `wx-config-2.8 --cflags` --> append of -2.8

wxWidgets wizard was tested several times on Linux before putting it in to repo. Why didn't you report the issue that time?

FYI, it works perfectly provided you select the appropriate options. Unless you specify exactly what happened with you (your OS, wx installation, errors, etc) I would consider that the wizard works perfectly.

It's a bad gesture from a developer to comment on other people's work by trying it once.

Remark : I never got the choice to say in I want pch or not !!!
Question : is that _BORLANDC_ pragma really needed. I just looks and feels plain ugly ?

Ok, noted. This will be added. _BORLANDC_ pragma is necessary if Borland C++ 5.5 compiler is used.

EDIT : I would suggest to get rid of that _BORLANDC_ thing, since CB code is purely targeted for GCC. And I really wonder if Borland still needs such an ugly beast.

Is it written anywhere? Or am I missing anything?

If you are talking about compiling C::B's code with Borland C++ 5.5 compiler, then you may remove that pragma. BCC 5.5 can't compile Squirrel.
« Last Edit: June 23, 2007, 07:57:30 am by Biplab »
Be a part of the solution, not a part of the problem.

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: new wxWidget(wxSmith) project
« Reply #4 on: June 23, 2007, 07:47:28 am »
For all users information, wxWidgets wizard is Now Broken.

It'll work with wxSmith. But NOT when you'll be using it without wxSmith.

Edit 1: @Byo, Thanks for the update.  :)  But the problem is wxSmith adds the necessary headers whereas without wxSmith the wizard generated codes become blind.

I'll update the wizard generated codes.

Regards,

Biplab
« Last Edit: June 23, 2007, 08:13:49 am by Biplab »
Be a part of the solution, not a part of the problem.

Offline mandrav

  • Project Leader
  • Administrator
  • Lives here!
  • *****
  • Posts: 4315
    • Code::Blocks IDE
Re: new wxWidget(wxSmith) project
« Reply #5 on: June 23, 2007, 08:32:30 am »
EDIT : I would suggest to get rid of that _BORLANDC_ thing, since CB code is purely targeted for GCC. And I really wonder if Borland still needs such an ugly beast.

Is it written anywhere? Or am I missing anything?

If you are talking about compiling C::B's code with Borland C++ 5.5 compiler, then you may remove that pragma. BCC 5.5 can't compile Squirrel.

Yes, he's talking about C::B's source code. It's only compilable with gcc.
Be patient!
This bug will be fixed soon...

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5493
Re: new wxWidget(wxSmith) project
« Reply #6 on: June 23, 2007, 10:39:14 am »
@Biplab : don't get me wrong, I am not putting anything or anyone down.

I had tried the wizard on windows, where I get a lot of choices (like #wx). So in linux I did similar. Now I didn't get that choice although I had set up that global variable correctly. So nothing compiled. By looking into the project options I saw the wx-config and then with some help of other people we saw that in the wx284 package we installed on OpenSuse it's called wx-config-2.8. Did you notice the [EDIT] part in the post explaining the issue was fixed !!
SO I looked for the solution and put it in the post, I am sorry there's nothing more that I can do. I wrote down my experience.
And then I started my rattle on the generated code, which can be improved as I suggested.

- don't include pch headers in a header file, which was what happened !!!
- other includes were missing (never depend on a.h including b.h, be clear, if you use wxString --> include wxString.h, I don't think a user should start checking if wx/intl.h includes wxstring.h).
[Meyers and Sutter and Alexandrescu] have some interesting topics in their books on headers.


You know our wizards work, wxSmith does splendid things, and if then the code that comes out of it is clean and good C++, then we have something very beautiful.

Cheers.

Offline byo

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 837
Re: new wxWidget(wxSmith) project
« Reply #7 on: June 23, 2007, 11:27:25 am »

It'll work with wxSmith. But NOT when you'll be using it without wxSmith.

Edit 1: @Byo, Thanks for the update.  :)  But the problem is wxSmith adds the necessary headers whereas without wxSmith the wizard generated codes become blind.


Sorry for that  :oops:. I don't know what I've done wrong but I also tested all three options for GUI designer and it compiled and run fine, I must have committed wrong script. Again sorry for the inconvenience.

Regards
   BYO

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: new wxWidget(wxSmith) project
« Reply #8 on: June 23, 2007, 11:31:48 am »
@Biplab : don't get me wrong, I am not putting anything or anyone down.

First of all, Sorry for my Hot comments. :roll:

I had tried the wizard on windows, where I get a lot of choices (like #wx). So in linux I did similar. Now I didn't get that choice although I had set up that global variable correctly. So nothing compiled.

On Linux this method is not followed which is why we have two project files for Code::Blocks. ;)

By looking into the project options I saw the wx-config and then with some help of other people we saw that in the wx284 package we installed on OpenSuse it's called wx-config-2.8. Did you notice the

It's strange that it didn't work for you. I have OpenSUSE 10.2 installed and that works for me. I'm not using any pre-compiled rpm. But I believe wx-config is the actual one to be used.

- don't include pch headers in a header file, which was what happened !!!

My vote for this one and thanks to Byo for the change. :)

- other includes were missing (never depend on a.h including b.h, be clear, if you use wxString --> include wxString.h, I don't think a user should start checking if wx/intl.h includes wxstring.h).

I would differ with you on this. :) The most commonly used headers are included through <wx/wx.h> file. wxString header is also included in that header. So if it is included we don't need to include <wx/string.h> again.

If you notice the dialog sample, you'll find two extra headers are included which are not included through <wx/wx.h> file.

You know our wizards work, wxSmith does splendid things, and if then the code that comes out of it is clean and good C++, then we have something very beautiful.

That's what we all want. :)

Best Regards,

Biplab :)
Be a part of the solution, not a part of the problem.

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: new wxWidget(wxSmith) project
« Reply #9 on: June 23, 2007, 11:37:44 am »
Sorry for that  :oops:. I don't know what I've done wrong but I also tested all three options for GUI designer and it compiled and run fine, I must have committed wrong script. Again sorry for the inconvenience.

You have made nice changes in the script and it's fine. :)

The problem is you've removed the following lines
Quote
#ifndef WX_PRECOMP
    #include <wx/wx.h>
#endif

Now wx.h includes a number of commonly used headers. So if it's not there, we've to include <wx/frame.h>, <wx/button.h>, etc for all the classes we want to use. There lies the problem. :)

Regards,

Biplab
Be a part of the solution, not a part of the problem.

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5493
Re: new wxWidget(wxSmith) project
« Reply #10 on: June 23, 2007, 11:51:35 am »
as said, I would suggest to be explicit and not just wx.h.
Things are clearer to the user also, he knows which headers are in play, and no need to search wx.h for which header it could be. Less surprises if certainly a header get's kicked out of wx.h. Hopefully this won't happen but you never know with these wx guys.

And secondly if you don't use pch, wx.h is bringing way to many things into play and will slow down compilation.

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: new wxWidget(wxSmith) project
« Reply #11 on: June 23, 2007, 11:57:17 am »
as said, I would suggest to be explicit and not just wx.h.
Things are clearer to the user also, he knows which headers are in play, and no need to search wx.h for which header it could be. Less surprises if certainly a header get's kicked out of wx.h. Hopefully this won't happen but you never know with these wx guys.

And secondly if you don't use pch, wx.h is bringing way to many things into play and will slow down compilation.

Got your point. I'll make the necessary changes. :)


@All, wxwidgets wizard is now working well with/without GUI designers.
Be a part of the solution, not a part of the problem.

Offline byo

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 837
Re: new wxWidget(wxSmith) project
« Reply #12 on: June 23, 2007, 12:37:27 pm »
The problem is you've removed the following lines
Quote
#ifndef WX_PRECOMP
    #include <wx/wx.h>
#endif

Now wx.h includes a number of commonly used headers. So if it's not there, we've to include <wx/frame.h>, <wx/button.h>, etc for all the classes we want to use. There lies the problem. :)

Now I get it :), haven't checked that with pch disabled. Lesson for me: alays consult wxWizard changes with you before applying.

Regards
   BYO

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: new wxWidget(wxSmith) project
« Reply #13 on: June 23, 2007, 01:18:22 pm »
Byo,

Another point. It seems that you've removed _BORLANDC_ preprocessor directive and the associated pragma.

Please note that this will affect the compilation of Pch enabled App with BCC.

BCC 5.5.1 can't generate Pre-Compiled Header from a Header file. :(  What it does is that it creates the PCH on the fly while compiling a cpp file. It includes all the headers found in a cpp file to the generated pch file. This pragma is necessary to tell BCC that - "Please don't include any header files inside the PCH file from now on." This prevents regeneration of PCH file while compiling a large app.

If you remove that pragma, then BCC will include all the headers found in that cpp file and will include them in PCH file (if they were not included earlier). This will slow down the compilation process.

I hope I could make my point clear to you. :)

IMHO, if you remove them, then you may have to tell users that wxSmith doesn't support pch creation with BCC 5.5. :)

Best Regards,

Biplab
« Last Edit: June 23, 2007, 01:20:26 pm by Biplab »
Be a part of the solution, not a part of the problem.

Offline byo

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 837
Re: new wxWidget(wxSmith) project
« Reply #14 on: June 23, 2007, 01:48:50 pm »
Another point. It seems that you've removed _BORLANDC_ preprocessor directive and the associated pragma.

Hmm, I was just wondering - there's this _BORLANDC_ stuff inside wx_pch.h file:

Code
#ifndef WX_PCH_H_INCLUDED
#define WX_PCH_H_INCLUDED

// basic wxWidgets headers
#include <wx/wxprec.h>

#ifdef __BORLANDC__
#pragma hdrstop
#endif

#ifndef WX_PRECOMP
#include <wx/wx.h>
#endif

#ifdef WX_PRECOMP
// put here all your rarely-changing header files
#endif // WX_PRECOMP

#endif // WX_PCH_H_INCLUDED

And when the project is created and uses wxSmith, it includes this wx_pch just at the beginning of each source. In fact it should work as you've said - it will only cache <wx/wxprec.h> header. So it should work correctly. And it really cleans the source. Maybe with one exception: the "// put here all your rarely-changing header files" content should be placed before _BORLANDC_ stuff to cache rarely-changing project headers too. The only problem with wxSmith is that this "wx_pch.h" is not included for newly created resources but I'll probably try to detect such file and add it automatically.

Same technique could be used for other GUI designers too so everything related to pch would be put inside wx_pch.h and there won't be any need of using those #pragma hdrstop in sources anymore. What do you think ?

Regards
   BYO

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: new wxWidget(wxSmith) project
« Reply #15 on: June 23, 2007, 02:25:19 pm »
Hmm, I was just wondering - there's this _BORLANDC_ stuff inside wx_pch.h file:

I missed it. :)

So it should work correctly. And it really cleans the source. Maybe with one exception: the "// put here all your rarely-changing header files" content should be placed before _BORLANDC_ stuff to cache rarely-changing project headers too. The only problem with wxSmith is that this "wx_pch.h" is not included for newly created resources but I'll probably try to detect such file and add it automatically.

This is a good idea. Or you may keep an option to specify the header file name (similar to wx_pch.h) which wxSmith will use. That'd be an easy option to implement.

"// put here all your rarely-changing header files" has to be pulled up if we want to support PCH for BCC. ;)

But AFAIK, newer BCC (Turbo C++ Explorer 2006's compiler) supports PCH creation using a Header file. :) So that bad hack is necessary only for BCC 5.5.

Other supported GUI builder, wxFormBuilder generates it's own source which will have that pragma. So I would not clean up code for that. :)

But definitely the clean-up would be good for wxSmith and w/o any gui builder.

Regards,

Biplab
Be a part of the solution, not a part of the problem.

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5493
Re: new wxWidget(wxSmith) project
« Reply #16 on: June 23, 2007, 02:31:42 pm »
ok , so now we know why that __BORLANDc__ thing is needed.

Well, it's a decision then to support Borland with pch or not.

But keep the following in mind. Say the some other compilers (Digital Mars or whatever) also need special stuff for pch in the code, then we can end up with a list of such #ifdef do whatever is needed in here for that specifi compiler #endif structures, and that will make the code less easier to read.
My point of view on this is, first we should have standard compliant code, so it's as portable as can be. Pch is not part of the standard (I think), that's why I am stressing on the point to do includes correct, because then you at least have correct standard code.
CB code has decided to use the pch feature of GCC, and therefor you find things like :
Code
#include "the pch header"
#ifndef CB_PRECOMP
blablabla
#endif
more blablabla
where blablabla and more blablabl is header inclusion directives.

So, support BorlandC : yes or no. CB sources itself *only* support gcc, so in CB sources that Borland stuff is ready to go out of the door.

When you want to enable Borland pch, it is sufficient to do it in that pch header (sdk.h), and for pch support that header should be included first. And then the blablabla stuff.
I think wx pch header probably does similar thing, so also that points that it is not needed to do in our sources or wizard generated sources.

Oh, new post already before this one is ready, with interesting information :
Quote
But AFAIK, newer BCC (Turbo C++ Explorer 2006's compiler) supports PCH creation using a Header file. So that bad hack is necessary only for BCC 5.5.

Bye bye BCC5.5 ;-)

When a user is creating a bigger project starting from the wizards code, it can happen he wants to to pch stuff with his code, well that's the users choice, I feel that the code generated from our wizards should NOT take that into account. We can not, and should not, predict all possible uses or ideas a user might have in mind.

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: new wxWidget(wxSmith) project
« Reply #17 on: June 23, 2007, 02:45:11 pm »
So, support BorlandC : yes or no. CB sources itself *only* support gcc, so in CB sources that Borland stuff is ready to go out of the door.

As I wrote earlier, you can't compile C::B with BCC 5.5.1 as it fails to compile Squirrel. I have a partial makefile which I can give you in case you want to try.

Bye bye BCC5.5 ;-)

We may need to say bye bye to this little compiler. Turbo package is full of S**t. You've to install the whole IDE + .NET SDK 1.1 to get the compiler working for you. :x

Quote
When a user is creating a bigger project starting from the wizards code, it can happen he wants to to pch stuff with his code, well that's the users choice, I feel that the code generated from our wizards should NOT take that into account. We can not, and should not, predict all possible uses or ideas a user might have in mind.

It's the most probable case that the user will use his/her own header file. We're just providing a basic framework. Now it's the user who'd take the final decision. :)

Regards,

Biplab
Be a part of the solution, not a part of the problem.

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5493
Re: new wxWidget(wxSmith) project
« Reply #18 on: June 23, 2007, 03:04:05 pm »
Quote
Turbo package is full of S**t. You've to install the whole IDE + .NET SDK 1.1 to get the compiler working for you.

Darn right, I hope one day, they will provide just the compiler tools.

Offline byo

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 837
Re: new wxWidget(wxSmith) project
« Reply #19 on: June 23, 2007, 04:32:31 pm »
One thing also bothers me abotu this wx_pch.h file. There's code:

Code
#ifndef WX_PRECOMP
#include <wx/wx.h>
#endif

which I think is wrong because valid source using pch should look like this:

Code
#include "wx_pch.h"

#ifndef WX_PRECOMP
// Include headers that are inside pch, may be even #include <wx/wx.h>
#endif
// Include headers that are not inside pch

So actually the code I've pointed out from wx_pch.h should be put into sources (actually currently such non-pch source is in both wx_pch.h and source files). So the content of wx_pch.h proposed by me is:

Code
#ifndef WX_PCH_H_INCLUDED
#define WX_PCH_H_INCLUDED

// basic wxWidgets headers
#include <wx/wxprec.h>

#ifdef WX_PRECOMP
// put here all your rarely-changing header files
#endif // WX_PRECOMP

#ifdef __BORLANDC__
#pragma hdrstop
#endif

#endif // WX_PCH_H_INCLUDED

That may speed-up non-pch builds since in most cases not all <wx/wx.h> is included but only those headers that are really required. And from what I've seen, currently it's done by some trick like:

Code
#ifdef WX_PRECOMP
#include "wx_pch.h"
#endif

So the #ifdef..#endif guard is not needed at all since wx_pch.h won't do anything when pch-s are not enabled


BYO

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: new wxWidget(wxSmith) project
« Reply #20 on: June 23, 2007, 05:28:02 pm »
That would be a nice change. :)

I'll update the Non-wxSmith generated sources without <wx/wx.h> header. I believe that would make the PCH file smaller and the build be faster, too.
Be a part of the solution, not a part of the problem.

Offline byo

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 837
Re: new wxWidget(wxSmith) project
« Reply #21 on: June 23, 2007, 05:45:21 pm »
That would be a nice change. :)

I'll update the Non-wxSmith generated sources without <wx/wx.h> header. I believe that would make the PCH file smaller and the build be faster, too.

I've already updated the wxSmith-only files. So there's only this wx_pch.h left to update (it's shared so I don't know if you also wanted to update this one), I'll leave this to you since there may still be some problems with non-wxSmith projects after pch change.

And it looks like wx projects start having beter and cleaner shape and even though, the PCH compatibility is granted :) Of course they still can be better but even now I like the result :D

BYO

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5493
Re: new wxWidget(wxSmith) project
« Reply #22 on: June 24, 2007, 10:19:31 am »
I think we have a little problem.

On linux there's no wx_pch.h, or I am not finding it.

I replace it by  :
Quote
#include "wx/wxprec.h"
and then it compiles again.
I didn't change anything in svn. What do you guys think ??

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: new wxWidget(wxSmith) project
« Reply #23 on: June 24, 2007, 10:35:34 am »
I think we have a little problem.

On linux there's no wx_pch.h, or I am not finding it.

I replace it by  :
Quote
#include "wx/wxprec.h"
and then it compiles again.

We need to check the wizard in Linux, too. :)

Including "wx/wxprec.h" would kill the benefits and cleanliness that you and Byo have proposed. But for a temporary fix, it can be used. :D
« Last Edit: June 24, 2007, 10:40:07 am by Biplab »
Be a part of the solution, not a part of the problem.

Offline byo

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 837
Re: new wxWidget(wxSmith) project
« Reply #24 on: June 24, 2007, 11:27:06 pm »
I think we have a little problem.

On linux there's no wx_pch.h, or I am not finding it.

I replace it by  :
Quote
#include "wx/wxprec.h"
and then it compiles again.

We need to check the wizard in Linux, too. :)

Including "wx/wxprec.h" would kill the benefits and cleanliness that you and Byo have proposed. But for a temporary fix, it can be used. :D

For unix-based wizard there's "wxWidgets Library Settings" with two options:
- Use default wxWidgets configuration
- Use Advanced Options

In the first mode all extra options (including pch) are disabled. But "Create Empty Project" and "Create and use precompiled header (PCH)" should also be enabled since it should be possible to create both empty and pch-enabled project even when default wx settings are used.

Including "wx/wxprec.h" is only a workaround and won't create real pch-s. That's because .gch file is not created (it's possible to create such file but it will require adding .gch file to system files which probably is not a best choice). So the pch option for unix is really required :)

BYO

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5493
Re: new wxWidget(wxSmith) project
« Reply #25 on: June 25, 2007, 07:48:12 am »
are you saying linux builds have to use pch ?
Well my suse boxes are by default non pch, so for example even my linux CB builds are non pch.
I guess I could turn it on somewhere, but not for me, I'll stick to standard way ;-)

Now back to the real problem : linux : no wx_pch.h !!

But looking at the contents of wx/wxprec.h, I feel that's the header to include. If pch is turned on, it brings all those headers, if not pch it remains nearly empty no bringing any headers, so this behavior looks ok for me. Am I missing something ?
« Last Edit: June 25, 2007, 07:49:52 am by killerbot »

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: new wxWidget(wxSmith) project
« Reply #26 on: June 25, 2007, 08:06:44 am »
For unix-based wizard there's "wxWidgets Library Settings" with two options:
- Use default wxWidgets configuration
- Use Advanced Options

In the first mode all extra options (including pch) are disabled. But "Create Empty Project" and "Create and use precompiled header (PCH)" should also be enabled since it should be possible to create both empty and pch-enabled project even when default wx settings are used.

It's a nice catch. This is a drawback of the present wizard. Thanks for pointing it. :)

I'll update this today.

are you saying linux builds have to use pch ?

No No, Byo's not meaning that. What he's saying is that the option to select PCH is turned off by default. And if you don't want to use Advanced settings to enable pch, then you may need to do it in that way. :)
Be a part of the solution, not a part of the problem.