Re: [lmi] Child documents

[Greg Chicares]

On 2010-05-13 14:53Z, Vadim Zeitlin wrote:
> On Thu, 13 May 2010 01:56:24 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> > GC> Compared to that "far more limiting" concept, what lmi uses is still
> GC> > GC> more limited, because it omits the Save[As]() part: all that 
> remains is
> GC> > GC> to "simply group TDocument [wxDocument] objects in a hierarchy" and 
> to
> GC> > GC> cascade only one operation, i.e., closing.
> GC> > 
> GC> >  I thought that this was indeed just a limitation and so it still made
> GC> > sense. However now I'm less sure: if a child document can't be saved
> GC> > independently of the main one
> GC> 
> GC> It can't be saved in any sense whatsoever.
> 
>  FWIW the implementation shown below doesn't really prevent saving child
> documents.

  http://review.bakefile.org/r/204/diff/#index_header
1094    (usually when it's being created itself). They also can't be 
independently
1095    saved. A child document has its own view with the corresponding window.

Then I think you're documenting that they can't be saved independently,
as a precaution to anyone who might hope to find support for that; and
also saying that you haven't created obstacles to prevent a determined
and clever user from circumventing that documentary restriction. If so,
then that sounds okay to me, as long as I can implement such a
restriction myself, e.g. with
  EVT_UPDATE_UI(wxID_SAVE...

> It doesn't do anything in my example however and I'm not sure if
> it can be reasonably implemented. But child documents are a rare concept
> and I think that 99% of their use will be read-only anyhow so I'll just
> postpone this problem for now. Except that it would be arguably reasonable
> to disable wxID_SAVE[_AS] menu items for child documents inside wx itself
> instead of doing it in your code.

It's okay with me if lmi does this for now, and then maybe someday wx
will take care of it. But today lmi has:

      EVT_UPDATE_UI(wxID_SAVE                 
,IllustrationView::UponUpdateFileSave)

  void IllustrationView::UponUpdateFileSave(wxUpdateUIEvent& e)
  {
      e.Enable(!is_phony_ && document().IsModified());
  }

which depends on IllustrationView::is_phony_, which depends on my
LMI_WX_CHILD_DOCUMENT kludge, which abuses the wx document-creation
flags--and maybe someday wx will use the 2^3 bit despite my saying
here that it "seems unlikely":

  /// WX !! The wx document-view implementation has no notion of 'child'
  /// documents, but sometimes lmi creates a document that logically is
  /// a 'child' of a parent CensusDocument: it corresponds to no actual,
  /// distinct document, can't be opened or saved separately, and should
  /// be closed, along with all its views, when its parent closes; and,
  /// accordingly, it should never be added to any wxFileHistory. This
  /// set of behaviors is implemented here by implicitly defining a new
  /// document-creation flag, appropriating an unused bit in the flags
  /// word. This is brittle, but then again it seems unlikely that
  /// anyone will change this aspect of wx.

  enum {LMI_WX_CHILD_DOCUMENT = 8};

Does this changeset add a non-brittle way to do that? Instead of
testing for a document-creation flag, would lmi access the parent
document member and check whether its child list contains the
present document? [Oh--I guess you answer this below: check for
m_documentParent==NULL.]

BTW, note the file-history comment above:
  /// accordingly, it should never be added to any wxFileHistory. This
Is that handled automatically already? I'm thinking that, if child
documents can't be saved, then they can't be opened either, so they
also can't be opened via file history.

> GC> > (and, in any case, can't be opened independently of it because the
> GC> > user never selects any parent document so creating/opening child
> GC> > documents is always done programmatically anyhow), what exactly can
> GC> > it be used for that just a view can't?
> GC> 
> GC> Yes: that is the question.
> 
>  I'm still not sure about the answer to this one. I did find one possible
> explanation of why you didn't do it like this however: even though wx
> docview framework [implicitly] claims to support multiple views for each
> document, in practice this doesn't work. There is too much code which is
> clearly broken if there is more than one view. So perhaps you did try to do
> it like this but abandoned this approach?

I was certainly aware that some such code existed in wx:
  [lmi/view_ex.hpp]
  // TODO ?? No provision is yet made here for updating views when
  // document data changes; wx provides some functions for that.
but I seem to have thought it might work:
  [lmi/main_wx.cpp]
  // TODO ?? CALCULATION_SUMMARY It would probably be in much better
  // taste to use wxView::OnUpdate() for this purpose.

As far as I can remember, I studied wx's doc-view implementation and
perceived it to be pretty compatible with OWL's, so I just kludged in
a LMI_WX_CHILD_DOCUMENT flag directly without trying any alternative
based on multiple views.

> GC> I suppose a separate document is a convenience rather than a necessity;
> GC> or at least it seems convenient to an old OWL hand.
> GC> 
> GC> Here's a complete description of the additional behavior I seek.
> GC>   File | New | Census  # creates window 'A'
> GC>   Census | Run case    # creates window 'B'
> GC>   Window | Next        # focuses window 'A'
> GC>   [MDI-menu] | Close   # closes only 'A', and that's the problem.
> GC> I want 'B' to close whenever 'A' closes for any reason.
> 
>  This does work with the "image details" child document from this patch:
> 
>       http://review.bakefile.org/r/204/diff/#index_header

I've read these parts:
  include/
  interface/
  src/common/
(but not sample/), and it looks like what I had in mind. (I didn't
try to find out what a wxDList is, or why you can't iterate over
it with for(;;), because I don't want to get mired in the details.)

> GC> At the same time, I want to preserve an existing behavior that I have
> GC> kludged in via the 'flags' argument of wxDocTemplate::CreateDocument():
> GC> or-ing in my own LMI_WX_CHILD_DOCUMENT causes IllustrationView::is_phony_
> GC> to be set to 'true', and then is_phony_ is used to disable numerous
> GC> commands that wouldn't make sense for the "child". All the "magic" is
> GC> really in MakeNewIllustrationDocAndView().
> 
>  You could just for m_documentParent==NULL, of course. And we probably
> should add an wxDocument::IsChild() accessor.
> 
>  IOW the patch above should be a good enough base for doing something like
> what you did in 
> https://savannah.nongnu.org/support/download.php?file_id=17697.
> In fact the next question I have is whether I should create a working patch
> doing this for LMI?

Yes, please, and let me know when the supporting changes are available
in a wx snapshot.

> GC> You might look at this as a horror rather than a convenience, and that's
> GC> okay--I just want these particular behaviors, and I'm happy if they can
> GC> be achieved by more tasteful means.
> 
>  I am not totally sure but I think that having a view would be a cleaner
> solution. The trouble is that I think child documents make sense if you
> allow modifying them because then having a separate document could be an
> advantage, e.g. it'd have its own "modified" status. But the only
> reasonable examples of their use I can come up with are all read-only. And
> in read-only case another view seems exactly like what we need.
> 
>  But in its current form the patch is simple enough to at least not do any
> active harm and should provide a working solution.

Yes, it seems to be just what we need. And the document-parent
member was already there anyway.

> While using another view
> would require me spending some more time on fixing the bugs related to
> multiple views in wx docview code (which I'd be glad to do but it's not
> trivial, see http://article.gmane.org/gmane.comp.lib.wxwidgets.devel/121678
> for the kind of horrors that lurk there currently).

Fascinating. I guess lmi should just steer clear of multiple views.

> GC> Felicitously enough, we're resuming this discussion just when we've
> GC> been talking about completely replacing the census-view class anyway.
> 
>  I'd really like to either finish child documents support soon or finally
> discard it and fix multiple views support instead, simply because this task
> is hanging over me for so long. I realize that this is not the best reason
> to hurry it from the project management point of view but if we/you could
> decide whether child document are needed or not, it would be really great.

Yes, let's get this resolved. Your wx patch is minimal and can't do any
harm to anyone else AFAICT, and it does seem to provide the support that
lmi needs to finish this task in a straightforward way.