Re: [lmi] product editor patch

[Greg Chicares]

On 2008-02-23 03:33Z, Greg Chicares wrote:
> On 2008-02-07 19:10Z, Vaclav Slavik wrote:
>> 
>> I submitted updated product editor patch here:
>> https://savannah.nongnu.org/patch/index.php?6411
> 
> On 20080217T1517Z I committed most of these changes to HEAD.
[...]
> I think HEAD now has essentially all the product-editor changes.
> 
> The only exception is 'stratified_charges.[ch]pp', where
[...]
> I believe I've incorporated every idea in your patch except the
> extra error-trapping layer in read_entity() and write_entity().
> There, I think I understand what you did and why you did it, but
> I'm not yet sure that's the best possible place to do it--maybe
> we should (someday, later on) look for a way to trap errors like
> these in the GUI, but that's a decision we can defer.

No end user has said anything about this, so let's defer any
further change in 'stratified_charges.[ch]pp' indefinitely.

>> It's against CVS HEAD, so it may contain some unapplied 
>> editor-unrelated patches
> 
> Yes: mostly small changes, to about seventeen files. I'm going to
> keep them in abeyance for now

I've been working through all of them, and have already committed
most. These are the few hunks that remain:

(A) mvc_controller.cpp
s/wxControlWithItems/wxItemContainer/
Please help me understand the benefit of this change.

(B) illustration_view.cpp
[add a wxBusyCursor in IllustrationView::Pdf()]
I tested this, but observed no busy cursor.

(C) main_wx.cpp
The as-yet-unapplied hunk is self documenting:

 void Skeleton::UponUpdateFileSave(wxUpdateUIEvent& event)
 {
     wxDocument* doc = doc_manager_->GetCurrentDocument();
-    event.Enable(doc && doc->IsModified());
+
+    // Note that the condition is (IsModified() || !GetDocumentSaved()).
+    // IsModified() is not enough since a new empty document
+    // is not modified, but yet has to have 'Save' menu item enabled.
+    // The solution is to check for !GetDocumentSaved().
+    event.Enable(doc && (doc->IsModified() || !doc->GetDocumentSaved()));

but I hesitate to apply it for two reasons:

(1) Should "File | Save" indeed be enabled for an unmodified new
document? I see two options:
  (a) disable it; or
  (b) enable it, but map it to "File | Save as".
I've always followed (a) and disabled it, but that's really just
a habit rather than a conscious choice. Is there a reason to
follow (b) instead? I've looked at various interface standards,
but can't find specific guidance. I see each of these behaviors
in more than one msw application. Does another platform mandate
one or the other?

(2) If we want to change from (1)(a) to (1)(b), then there are
other source files that'll need to be changed as well.

(D) inputillus_xml_io.cpp, ledger_xml_io.cpp, xml_lmi.cpp
Apparently the purpose of these changes is to avoid writing
empty xml child text nodes. Can you give me an example of the
problem that this change would solve? Is it simply a matter of
writing '<X/>' instead of '<X></X>' because the former seems
more tasteful?