Opened 2 years ago

Closed 2 years ago

#14785 closed enhancement (fixed)

protect definitions in event.h interface file respectively to their wxlib

Reported by: phi Owned by:
Priority: normal Milestone:
Component: documentation Version: stable-latest
Keywords: Cc:
Blocked By: Blocking:
Patch: yes

Description

Similar to #14784.
To be able to run SWIG for each wxlib, it is easier to have them in separate files.

Attachments (2)

fix_event_doc.patch download (111.0 KB) - added by phi 2 years ago.
fix_event_doc_preprocessor.patch download (21.1 KB) - added by phi 2 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 2 years ago by vadz

  • Status changed from new to infoneeded_new

Same questions as for the other one:

  1. Is it really necessary to just move things around (wxCommandEvent in this case)?
  2. Could you please add @header{wx/event.h} to the stuff in event_base.h?

TIA!

comment:2 Changed 2 years ago by phi

  • Status changed from infoneeded_new to new

Hi,

Well for this one, I didn't moved anything. Though the patch looks ugly, so I re-run patch with -d which outputs a far more readable patch.

I updated also the missing @header

Should be ok now.

Regards

Changed 2 years ago by phi

comment:3 Changed 2 years ago by VZ

  • Resolution set to fixed
  • Status changed from new to closed

(In [72817]) Split documentation of non-GUI wxEvent-related classes.

Separate GUI from base classes to make it easier to generate wrappers for just
the latter using e.g. SWIG.

Closes #14785.

comment:4 Changed 2 years ago by robind

  • Resolution fixed deleted
  • Status changed from closed to reopened

Unfortunately this just causes a new problem. Now the Doxygen xml is being produced as if there is a real wx/event_base.h file, and since there isn't one then code generated from that XML gets compilation errors. See http://buildbot.wxpython.org:8010/builders/build-gtk-py27/builds/607/steps/shell/logs/stdio

comment:5 follow-up: Changed 2 years ago by robind

Oops, clicked submit too soon...

I can probably work around the missing header file if need be, but IMO it would be much better to keep the interface header files matching the real include files, as I think that this will affect the HTML docs produced as well and will document it as "#include <wx/event_base.h>" there too. So we either need to have a real event_base.h in include/wx, or there needs to be some other way to identify which lib the classes are in that can be used by code generators, or the OP can just bite the bullet and assume that wrappers for base and core classes will always be in combined in the resulting target extension modules.

Thoughts? Better ideas?

comment:6 in reply to: ↑ 5 Changed 2 years ago by robind

Replying to robind:

I think that this will affect the HTML docs produced as well and will document it as "#include <wx/event_base.h>" there too.

Confirmed. Although the @header tag will do the right thing lower down in the document, there is still a "#include <wx/event_base.h>" at the top of the document for the classes coming from event_base.h.

comment:7 follow-up: Changed 2 years ago by vadz

My only suggestion is to use #if wxUSE_GUI tests in the interface headers, just as we use them in the real ones. Officially having wx/event_base.h is IMHO ugly.

I don't understand why doesn't @header work though...

Anyhow, should I revert this (and what about #14784?) or will you do this?

comment:8 in reply to: ↑ 7 ; follow-up: Changed 2 years ago by phi

Replying to vadz:

My only suggestion is to use #if wxUSE_GUI tests in the interface headers, just as we use them in the real ones. Officially having wx/event_base.h is IMHO ugly.

Yeah, I'll create a new patch with these preprocessor conditionals.

Anyhow, should I revert this (and what about #14784?) or will you do this?

I think it's the best way, so may anyone revert it.
Concerning #14784, I think we may keep it because the real header wx/generic/logg.h exists. @robind could you check the doxygen generation with this ticket reverted but keeping the #14784 ?

Once again, thanx to everyone for your quick replies.

Regards

comment:9 Changed 2 years ago by VZ

(In [72825]) Revert "Split documentation of non-GUI wxEvent-related classes."

This reverts r72817 as it resulted in build problems for wxPython and
incorrect headers in the generated documentation.

See #14785.

comment:10 in reply to: ↑ 8 ; follow-up: Changed 2 years ago by robind

Replying to phi:

Concerning #14784, I think we may keep it because the real header wx/generic/logg.h exists.

From my perspective, yes that is okay. As long as the file is really there then my build is happy. But...

@robind could you check the doxygen generation with this ticket reverted but keeping the #14784 ?

There is still the issue of having a different header file shown at the top of the document than what is showing in the Detailed Description section of the class documentation. See http://docs.wxwidgets.org/trunk/classwx_log_text_ctrl.html

comment:11 in reply to: ↑ 10 Changed 2 years ago by phi

Replying to robind:

There is still the issue of having a different header file shown at the top of the document than what is showing in the Detailed Description section of the class documentation. See http://docs.wxwidgets.org/trunk/classwx_log_text_ctrl.html

Yeah, I didn't check the real header I thought that users have to specifically include <wx/generic/logg.h> when they wanted GUI classes, but as I read the wx/log.h I saw the preprocessor conditional that includes it.

So please revert my patch for #14784, I'll submit a new one with preprocessor conditionals.

Regards

comment:12 Changed 2 years ago by phi

  • Summary changed from Split event.h interface file so that each file addresses a unique wxlib to protect definitions in event.h interface file respectively to their wxlib

Here is a new patch that introduces preprocessor condtionals to protect definitions using wxUSE_BASE and wxUSE_GUI.

Note also that the wxCommandEvent class has moved up due to subclasses being defined before it.

I don't know if something special has to be done in the doxygen config regarding the preprocessor conditionals. Someone on this?

Regards

comment:13 follow-up: Changed 2 years ago by vadz

I don't know neither, could you please run doxygen (see docs/doxygen/regen.sh (or .bat)) and check if all classes still appear in the output? If not, we probably need to add wxUSE_GUI=1 to PREDEFINED in docs/doxygen/Doxyfile_inc`.

comment:14 in reply to: ↑ 13 Changed 2 years ago by phi

Replying to vadz:

I don't know neither, could you please run doxygen (see docs/doxygen/regen.sh (or .bat)) and check if all classes still appear in the output? If not, we probably need to add wxUSE_GUI=1 to PREDEFINED in docs/doxygen/Doxyfile_inc`.

It's good we checked, doxygen is preprocessor aware. So as you told, I put wxUSE_BASE and wxUSE_GUI in PREDEFINED. I updated the patch so now the docs are generating ok.

Regards

Changed 2 years ago by phi

comment:15 Changed 2 years ago by VZ

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [72910]) Add wxUSE_{BASE,GUI} checks to interface headers.

This allows to define just one of them to run some tool, e.g. SWIG, on only
the classes defined in the base or in the core library (both are defined by
default for Doxygen itself).

Closes #14785.

Note: See TracTickets for help on using tickets.