Re: [lmi] Pasting between multiline edit controls

[Greg Chicares]

On 2015-10-10 00:48, Vadim Zeitlin wrote:
> On Thu, 08 Oct 2015 23:49:01 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> Pasting from one multiline edit control to another causes newlines to be
> GC> translated to semicolons--because that's what pasting from the clipboard
> GC> does in lmi--because we deliberately coded it that way--because end users
> GC> wanted to paste from spreadsheet columns into input-sequence fields, but
> GC> we didn't want to restrict it to such fields--because at that time we did
> GC> not distinguish such (input-sequence) fields.
> 
>  I didn't realize this. I noticed the behaviour you describe too and even
> found the reason for it in the code, but after seeing the comment about
> intercepting this event for "all windows" I just assumed there must have
> been a reason for it (e.g. that there were different kinds of windows for
> which this replacement made sense) and didn't go further than that.

/// At least for now, this transformation is performed iff the paste
/// target is a wxTextCtrl.

Poor comments like that only describe what the code does, without
explaining why.

/// Paste "\n"- or "\r\n"-delimited clipboard contents into a control.

Even worse comments, like that, describe what the code does, but not
correctly ("\t" is also respected as a delimiter).

The patch below revises this comment to say what I'd like the code to do;
but I don't know how to accomplish it.

> GC>   /// At least for now, this transformation is performed iff the paste
> GC>   /// target is a wxTextCtrl.
> GC> 
> GC>   void Skeleton::UponPaste(wxClipboardTextEvent& event)
> GC>   {
> GC> ...
> GC>       wxTextCtrl* t = dynamic_cast<wxTextCtrl*>(event.GetEventObject());
> GC> 
> GC> I'd like to consider a change like
> GC> -      wxTextCtrl* t = dynamic_cast<wxTextCtrl*>(event.GetEventObject());
> GC> +      wxTextCtrl* t = 
> dynamic_cast<InputSequenceEntry*>(event.GetEventObject());
> GC> 
> GC> But wxTextCtrl is not the base class (as I naively anticipated it to 
> be)...
> GC> 
> GC>   class InputSequenceEntry: public wxPanel
> GC> NOT wxTextCtrl.....................^^^^^^^
> 
>  InputSequenceEntry is a composite control, so while it contains a
> wxTextCtrl, is is not a wxTextCtrl.

Okay, the contained control should be accessible via
  wxTextCtrl& InputSequenceEntry::text_ctrl()

> GC> ...and for all I know this might be a better place to make a change...
> 
>  If we only need the replacements of newlines (and TABs) with semicolons to
> happen in InputSequenceEntry, the best would clearly be to move the logic
> for doing this inside this control itself.

Agreed. First I'd like to change it in place, to see whether I can do that
successfully (see below). It can be moved later.

> GC> ...so we'll work more efficiently if I stick to my own specialty, and
> GC> ask for help here, noting by the way that we don't want to break this:
> GC>   void Skeleton::UponTestPasting(wxCommandEvent&)
> GC>   {
> GC>       wxTextCtrl* t = new wxTextCtrl(frame_, wxID_ANY, "Testing...");
> GC>       ^^^^^^^^^^          ^^^^^^^^^^
> 
>  But, of course, the above would break this test case. IMHO the best would
> be to just replace wxTextCtrl above with an InputSequenceEntry: after all,
> if we want the replacements to work only in the latter, it doesn't make
> any sense to test that they work in the former. Is there any good reason
> not to do it?

I'm trying, but I'm stuck (see below).

>  Also, perhaps this should be moved out of Skeleton itself and into the GUI
> test suite?

Agreed: Skeleton::UponTestPasting() should be moved to the GUI test suite.
We can do that any time--it doesn't have to be right now.

But I can't get the paste handler to work. Let me present my patch and ask
you to tell me what I'm doing wrong.

Index: skeleton.cpp
===================================================================
--- skeleton.cpp        (revision 6400)
+++ skeleton.cpp        (working copy)
@@ -899,6 +899,7 @@
             }

         new_text.resize(1 + new_text.find_last_not_of(';'));
+        // TODO ?? Also remove any leading semicolons.

         return new_text;
         }
@@ -905,23 +906,37 @@

 } // Unnamed namespace.

-/// Paste "\n"- or "\r\n"-delimited clipboard contents into a control,
-/// replacing nonterminal delimiters with semicolons to form an input
-/// sequence. The motivation is to permit pasting spreadsheet columns.
+// [these comments describe desired behavior, not actual current behavior]
+
+/// Handle 'Text Paste' events for all windows.
 ///
-/// At least for now, this transformation is performed iff the paste
-/// target is a wxTextCtrl.
+/// The behavior depends upon the event-object type.
+///
+/// Type InputSequenceEntry: Paste "\n"-, "\r\n"-, or "\t"-delimited
+/// clipboard contents into the associated text control, replacing
+/// nonterminal delimiters with semicolons to form an input sequence,
+/// and removing any terminal delimiters. The motivation is to permit
+/// pasting spreadsheet columns.
+///
+/// Type wxTextCtrl: Paste literal clipboard contents. Text delimited
+/// with "\n" or "\r\n" is shown on distinct lines in a multiline text
+/// control with all delimiters removed; in single-line text controls,
+/// all delimiters are replaced by RETURN_SYMBOL.
+///
+/// All other types: ignore the paste event.

 void Skeleton::UponPaste(wxClipboardTextEvent& event)
 {
     event.Skip();

-    wxTextCtrl* t = dynamic_cast<wxTextCtrl*>(event.GetEventObject());
-    if(!t)
+    InputSequenceEntry* z = 
dynamic_cast<InputSequenceEntry*>(event.GetEventObject());
+    if(!z)
         {
         return;
         }

+    wxTextCtrl& t = z->text_ctrl();
+
     std::string const s(ClipboardEx::GetText());
     if(s.empty())
         {
@@ -928,7 +943,7 @@
         return;
         }

-    t->WriteText(redelimit_with_semicolons(s));
+    t.WriteText(redelimit_with_semicolons(s));
     event.Skip(false);
 }

@@ -1086,26 +1101,27 @@

 void Skeleton::UponTestPasting(wxCommandEvent&)
 {
-    wxTextCtrl* t = new wxTextCtrl(frame_, wxID_ANY, "Testing...");
-    LMI_ASSERT(t);
+    InputSequenceEntry* z = new InputSequenceEntry(frame_, wxID_ANY, 
"Testing...");
+    LMI_ASSERT(z);
+    wxTextCtrl& t = z->text_ctrl();

     ClipboardEx::SetText("1\r\n2\r\n3\r\n");
-    t->SetSelection(-1L, -1L);
-    t->Paste();
-    if("1;2;3" != t->GetValue())
+    t.SetSelection(-1L, -1L);
+    t.Paste();
+    if("1;2;3" != t.GetValue())
         {
-        warning() << "'1;2;3' != '" << t->GetValue() << "'" << LMI_FLUSH;
+        warning() << "'1;2;3' != '" << t.GetValue() << "'" << LMI_FLUSH;
         }

     ClipboardEx::SetText("X\tY\tZ\t");
-    t->SetSelection(-1L, -1L);
-    t->Paste();
-    if("X;Y;Z" != t->GetValue())
+    t.SetSelection(-1L, -1L);
+    t.Paste();
+    if("X;Y;Z" != t.GetValue())
         {
-        warning() << "'X;Y;Z' != '" << t->GetValue() << "'" << LMI_FLUSH;
+        warning() << "'X;Y;Z' != '" << t.GetValue() << "'" << LMI_FLUSH;
         }

-    t->Destroy();
+    t.Destroy();
     status() << "Pasting test finished." << std::flush;
 }