Re: commit (HEAD): bidi fixes and toolbar

From: William Lachance (william.lachance@sympatico.ca)
Date: Mon Jan 27 2003 - 00:56:10 EST

  • Next message: Hubert Figuiere: "Commit (HEAD): warning fix"

    On Sat, 2003-01-25 at 15:54, Tomas Frydrych wrote:
    > Hi Jeremy,
    >
    > sorry if the comments came across harsh; I have spent several
    > hours today trying to work out why the toolbar would not respond to
    > notification messages from the view. At the end I discovered that in
    > the win32 frame there vere some vectors declared that that had the
    > same names as vectors the xap impl class but got never used, and
    > consequently the win32 frame was under the impression it did not
    > have any toolbars. So I have deleted those vectors, at which point it
    > broke badly, because the frame cannot access the members of the
    > impl, since it is not its base class. That's why I had to move the
    > code for _bindToolbars to the impl, but that was not enought -- I
    > shuffled stuff around immitating the Unix frame, until I got it work.
    >
    > I have to say honestly that I do not see what we gained by having
    > Frame and FrameImpl classes, except a maintanance nightmare; I
    > see no clear conceptual distinction between the two, and trying to
    > find out what is done from where is difficult (not just in the win32),
    > but perhaps I am just ignorant :).
    >
    > Tomas

    Hi Tomas,

    The purpose was to consolidate code that is identical across platforms.
    So instead of cutting and pasting the same code in an AP_XXXFrame class,
    it can simply be declared once in an AP_Frame class.

    I have to confess that the job I did wasn't perfect. Chalk it up to
    relative inexperience with C++. I hope to work on improving the
    interface/implementation of the frame code over the coming months.

    Here's the verbatim text that I sent to the list:

    Unix-only for now, conditional compilation of XAP_Frame for other
    platforms. This requires some explanation:

    As some of you know, the old frame inheritance hierarchy was a bit of a
    mess.

    We had
    XAP_Frame -> XAP_XXXFrame -> AP_XXXFrame

    (where XXX is Win32, Unix, Qnx)

    This resulted in plenty of duplicated code in AP_XXXFrame, because we
    couldn't properly place stuff that wasn't platform independant in an
    application independant AP_Frame class.

    The new code in CVS is the start of a new frame hierarchy. Now we have:

    XAP_Frame -> AP_Frame -> AP_XXXFrame

    and

    XAP_FrameHelper --> XAP_XXXFrameHelper -> AP_XXXFrameHelper

    XAP_Frame, AP_Frame, now provide platform independant interfaces and
    implementations of stuff "that frames should do" (show toolbars, create
    a document window, etc.)

    XAP_FrameHelper, XAP_XXXFrameHelper, AP_XXXFrameHelper: This is where
    the implementation of platform-dependant services (or the virtual
    declarations for them, in the case of XAP_FrameHelper) should go.

    The good news: This will ultimately result in less duplicated code (I
    haven't started placing platform-independant abi frame code into
    AP_Frame class, I will soon!) and better model-view-controller
    seperation (it should be somewhat easier to grok the frame code when
    this is all finished).

    The bad news: There is some ugly casting going on and there were times
    where I had to expose the XAP_FrameHelper class to stuff which I'd
    rather not expose it to (e.g.: ev_UnixToolbar needs access to the gtk
    toplevel widget, which is now in XAP_UnixFrameHelper).

    Overall, I think this is a win. I will continue to polish the XP and
    UNIX code to make it better. I will need help from platform maintainers
    to bring the Win32/QNX code to this new system.

    -- 
    William Lachance <william.lachance@sympatico.ca>
    


    This archive was generated by hypermail 2.1.4 : Mon Jan 27 2003 - 00:55:17 EST