Split wxTextCompleter into a base class and wxTextCompleterSimple.

Allow overriding either the iterator-like methods of the base class or the
single and possibly more convenient, albeit slightly less efficient, method of
the derived wxTextCompleterSimple class.

This makes it possible to completely delegate to wxTextCompleter from wxMSW
IEnumString implementation and actually makes the code there easier, even if
it it still not quite trivial because of multi-threading concerns.

It also would make it possible to show the completions progressively, as they
are retrieved, in a future generic implementation of auto-completion (MSW
native implementation doesn't do this unfortunately and waits until all of the
completions become available before showing them).

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@67515 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

include/wx/textcompleter.h

@@ -20,7 +20,12 @@ class WXDLLIMPEXP_CORE wxTextCompleter
 public:
     wxTextCompleter() { }
 
-    virtual void GetCompletions(const wxString& prefix, wxArrayString& res) = 0;
+    // The virtual functions to be implemented by the derived classes: the
+    // first one is called to start preparing for completions for the given
+    // prefix and, if it returns true, GetNext() is called until it returns an
+    // empty string indicating that there are no more completions.
+    virtual bool Start(const wxString& prefix) = 0;
+    virtual wxString GetNext() = 0;
 
     virtual ~wxTextCompleter();
 
@@ -28,4 +33,27 @@ private:
     wxDECLARE_NO_COPY_CLASS(wxTextCompleter);
 };
 
+// ----------------------------------------------------------------------------
+// wxTextCompleterSimple: returns the entire set of completions at once
+// ----------------------------------------------------------------------------
+
+class WXDLLIMPEXP_CORE wxTextCompleterSimple : public wxTextCompleter
+{
+public:
+    wxTextCompleterSimple() { }
+
+    // Must be implemented to return all the completions for the given prefix.
+    virtual void GetCompletions(const wxString& prefix, wxArrayString& res) = 0;
+
+    virtual bool Start(const wxString& prefix);
+    virtual wxString GetNext();
+
+private:
+    wxArrayString m_completions;
+    unsigned m_index;
+
+    wxDECLARE_NO_COPY_CLASS(wxTextCompleterSimple);
+};
+
 #endif // _WX_TEXTCOMPLETER_H_
+

interface/wx/textcompleter.h

@@ -21,6 +21,59 @@
     hierarchically, i.e. only the first component initially, then the second
     one after the user finished entering the first one and so on.
 
+    When inheriting from this class you need to implement its two pure virtual
+    methods. This allows to return the results incrementally and may or not be
+    convenient depending on where do they come from. If you prefer to return
+    all the completions at once, you should inherit from wxTextCompleterSimple
+    instead.
+
+    @since 2.9.2
+ */
+class wxTextCompleter
+{
+public:
+    /**
+        Function called to start iteration over the completions for the given
+        prefix.
+
+        This function could start a database query, for example, if the results
+        are read from a database.
+
+        Notice that under some platforms (currently MSW only) it is called from
+        another thread context and so the appropriate synchronization mechanism
+        should be used to access any data also used by the main UI thread.
+
+        @param prefix
+            The prefix for which completions are to be generated.
+        @return
+            @true to continue with calling GetNext() or @false to indicate that
+            there are no matches and GetNext() shouldn't be called at all.
+     */
+    virtual bool Start(const wxString& prefix) = 0;
+
+    /**
+        Called to retrieve the next completion.
+
+        All completions returned by this function should start with the prefix
+        passed to the last call to Start().
+
+        Notice that, as Start(), this method is called from a worker thread
+        context under MSW.
+
+        @return
+            The next completion or an empty string to indicate that there are
+            no more of them.
+     */
+    virtual wxString GetNext() = 0;
+};
+
+/**
+    A simpler base class for custom completer objects.
+
+    This class may be simpler to use than the base wxTextCompleter as it allows
+    to implement only a single virtual method instead of two of them (at the
+    price of storing all completions in a temporary array).
+
     Here is a simple example of a custom completer that completes the names of
     some chess pieces. Of course, as the total list here has only four items it
     would have been much simpler to just specify the array containing all the
@@ -28,7 +81,7 @@
     total number of completions is much higher provided the number of
     possibilities for each word is still relatively small:
     @code
-    class MyTextCompleter : public wxTextCompleter
+    class MyTextCompleter : public wxTextCompleterSimple
     {
     public:
         virtual void GetCompletions(const wxString& prefix, wxArrayString& res)
@@ -60,7 +113,7 @@
 
     @since 2.9.2
 */
-class wxTextCompleter
+class wxTextCompleterSimple : public wxTextCompleter
 {
 public:
     /**
@@ -75,8 +128,8 @@ public:
         in the first place useless.
 
         Notice that this function may be called from thread other than main one
-        (this is currently always the case under MSW) so care should be taken
+        (this is currently always the case under MSW) so the appropriate
-        if it needs to access any shared data.
+        synchronization mechanism should be used to protect the shared data.
 
         @param prefix
             The possibly empty prefix that the user had already entered.

samples/widgets/widgets.cpp

@@ -1006,7 +1006,7 @@ void WidgetsFrame::OnAutoCompleteCustom(wxCommandEvent& WXUNUSED(event))
     // known. This allows to avoid building the full 676000 item list of
     // possible strings all at once as the we have 1000 possibilities for the
     // first word (000..999) and 676 (aa..zz) for the second one.
-    class CustomTextCompleter : public wxTextCompleter
+    class CustomTextCompleter : public wxTextCompleterSimple
     {
     public:
         virtual void GetCompletions(const wxString& prefix, wxArrayString& res)

src/common/textentrycmn.cpp

@@ -377,6 +377,23 @@ wxTextCompleter::~wxTextCompleter()
 {
 }
 
+bool wxTextCompleterSimple::Start(const wxString& prefix)
+{
+    m_index = 0;
+    m_completions.clear();
+    GetCompletions(prefix, m_completions);
+
+    return !m_completions.empty();
+}
+
+wxString wxTextCompleterSimple::GetNext()
+{
+    if ( m_index == m_completions.size() )
+        return wxString();
+
+    return m_completions[m_index++];
+}
+
 bool wxTextEntryBase::DoAutoCompleteCustom(wxTextCompleter *completer)
 {
     // We don't do anything here but we still need to delete the completer for

src/msw/textentry.cpp

@@ -100,8 +100,7 @@ private:
 } // anonymity namespace
 
 // Implementation of string enumerator used by wxTextAutoCompleteData. This
-// class simply iterates over its strings except that it also can be told to
+// class simply forwards to wxTextCompleter associated with it.
-// update them from the custom completer if one is used.
 //
 // Notice that Next() method of this class is called by IAutoComplete
 // background thread and so we must care about thread safety here.
@@ -111,36 +110,35 @@ public:
     wxIEnumString()
     {
         Init();
-
-        m_index = 0;
     }
 
-    wxIEnumString(const wxIEnumString& other)
+    void ChangeCompleter(wxTextCompleter *completer)
-        : m_strings(other.m_strings),
-          m_index(other.m_index)
     {
-        Init();
+        // Indicate to Next() that it should bail out as soon as possible.
+        {
+            CSLock lock(m_csRestart);
+
+            m_restart = TRUE;
         }
 
-    void ChangeStrings(const wxArrayString& strings)
+        // Now try to enter this critical section to ensure that Next() doesn't
-    {
+        // use the old pointer any more before changing it (this is vital as
-        CSLock lock(m_csStrings);
+        // the old pointer will be destroyed after we return).
+        CSLock lock(m_csCompleter);
 
-        m_strings = strings;
-        m_index = 0;
-    }
-
-    void UpdateStringsFromCompleter(wxTextCompleter *completer,
-                                    const wxString& prefix)
-    {
-        CSLock lock(m_csUpdate);
-
-        // We simply store the pointer here and will really update during the
-        // next call to our Next() method as we want to call GetCompletions()
-        // from the worker thread to prevent the main UI thread from blocking
-        // while the completions are generated.
         m_completer = completer;
+    }
+
+    void UpdatePrefix(const wxString& prefix)
+    {
+        CSLock lock(m_csRestart);
+
+        // We simply store the prefix here and will really update during the
+        // next call to our Next() method as we want to call Start() from the
+        // worker thread to prevent the main UI thread from blocking while the
+        // completions are generated.
         m_prefix = prefix;
+        m_restart = TRUE;
     }
 
     virtual HRESULT STDMETHODCALLTYPE Next(ULONG celt,
@@ -156,16 +154,22 @@ public:
 
         *pceltFetched = 0;
 
-        CSLock lock(m_csStrings);
+        CSLock lock(m_csCompleter);
 
-        DoUpdateIfNeeded();
+        if ( !RestartIfNeeded() )
-
-        for ( const unsigned count = m_strings.size(); celt--; ++m_index )
-        {
-            if ( m_index == count )
             return S_FALSE;
 
-            const wxWX2WCbuf wcbuf = m_strings[m_index].wc_str();
+        while ( celt-- )
+        {
+            // Stop iterating if we need to update completions anyhow.
+            if ( m_restart )
+                return S_FALSE;
+
+            const wxString s = m_completer->GetNext();
+            if ( s.empty() )
+                return S_FALSE;
+
+            const wxWX2WCbuf wcbuf = s.wc_str();
             const size_t size = (wcslen(wcbuf) + 1)*sizeof(wchar_t);
             void *olestr = CoTaskMemAlloc(size);
             if ( !olestr )
@@ -183,12 +187,20 @@ public:
 
     virtual HRESULT STDMETHODCALLTYPE Skip(ULONG celt)
     {
-        CSLock lock(m_csStrings);
+        if ( !celt )
+            return E_INVALIDARG;
 
-        m_index += celt;
+        CSLock lock(m_csCompleter);
-        if ( m_index > m_strings.size() )
+
+        if ( !RestartIfNeeded() )
+            return S_FALSE;
+
+        while ( celt-- )
         {
-            m_index = m_strings.size();
+            if ( m_restart )
+                return S_FALSE;
+
+            if ( m_completer->GetNext().empty() )
                 return S_FALSE;
         }
 
@@ -197,22 +209,9 @@ public:
 
     virtual HRESULT STDMETHODCALLTYPE Reset()
     {
-        {
+        CSLock lock(m_csRestart);
-            CSLock lock(m_csUpdate);
 
-            if ( m_completer )
+        m_restart = TRUE;
-            {
-                // We will update the string from completer soon (or maybe are
-                // already in process of doing it) so don't do anything now, it
-                // is useless at best and could also result in a deadlock if
-                // m_csStrings is already locked by Next().
-                return S_OK;
-            }
-        }
-
-        CSLock lock(m_csStrings);
-
-        m_index = 0;
 
         return S_OK;
     }
@@ -222,11 +221,13 @@ public:
         if ( !ppEnum )
             return E_POINTER;
 
-        CSLock lock(m_csStrings);
+        CSLock lock(m_csCompleter);
-
-        wxIEnumString *e = new wxIEnumString(*this);
 
+        wxIEnumString * const e = new wxIEnumString;
         e->AddRef();
+
+        e->ChangeCompleter(m_completer);
+
         *ppEnum = e;
 
         return S_OK;
@@ -241,79 +242,79 @@ private:
     // (mistakenly) delete us directly instead of calling Release()
     virtual ~wxIEnumString()
     {
-        ::DeleteCriticalSection(&m_csStrings);
+        ::DeleteCriticalSection(&m_csRestart);
-        ::DeleteCriticalSection(&m_csUpdate);
+        ::DeleteCriticalSection(&m_csCompleter);
     }
 
     // Common part of all ctors.
     void Init()
     {
-        ::InitializeCriticalSection(&m_csUpdate);
+        ::InitializeCriticalSection(&m_csCompleter);
-        ::InitializeCriticalSection(&m_csStrings);
+        ::InitializeCriticalSection(&m_csRestart);
 
         m_completer = NULL;
+        m_restart = FALSE;
     }
 
-    void DoUpdateIfNeeded()
+    // Restart completions generation if needed. Should be only called from
+    // inside m_csCompleter.
+    //
+    // If false is returned, it means that there are no completions and that
+    // wxTextCompleter::GetNext() shouldn't be called at all.
+    bool RestartIfNeeded()
+    {
+        bool rc = true;
+        for ( ;; )
         {
-        wxTextCompleter *completer;
             wxString prefix;
+            LONG restart;
             {
-            CSLock lock(m_csUpdate);
+                CSLock lock(m_csRestart);
-
-            completer = m_completer;
-            if ( !completer )
-                return;
 
                 prefix = m_prefix;
-        } // Unlock m_csUpdate to allow the main thread to call our
+                restart = m_restart;
-          // UpdateStringsFromCompleter() without blocking while we are
-          // generating completions.
 
-        // Notice that m_csStrings is locked by our caller already so no need
+                m_restart = FALSE;
-        // to enter it again.
+            } // Release m_csRestart before calling Start() to avoid blocking
-        m_index = 0;
+              // the main thread in UpdatePrefix() during its execution.
-        m_strings.clear();
-        completer->GetCompletions(prefix, m_strings);
 
-        {
+            if ( !restart )
-            CSLock lock(m_csUpdate);
+                break;
 
-            if ( m_completer == completer && m_prefix == prefix )
+            rc = m_completer->Start(prefix);
-            {
-                // There were no calls to UpdateStringsFromCompleter() while we
-                // generated the completions, so our completions are still
-                // pertinent.
-                m_completer = NULL;
-                return;
-            }
-            //else: Our completions are already out of date, regenerate them
-            //      once again.
         }
+
+        return rc;
     }
 
 
-    // Critical section protecting m_strings and m_index.
+    // Critical section protecting m_completer itself. It must be entered when
-    CRITICAL_SECTION m_csStrings;
+    // using the pointer to ensure that we don't continue using a dangling one
+    // after it is destroyed.
+    CRITICAL_SECTION m_csCompleter;
 
-    // The strings we iterate over and the current iteration index.
+    // The completer we delegate to for the completions generation. It is never
-    wxArrayString m_strings;
+    // NULL after the initial ChangeCompleter() call.
-    unsigned m_index;
-
-    // This one protects just m_completer, it is different from m_csStrings
-    // because the main thread should be able to update our completer prefix
-    // without blocking (as this would freeze the UI) even while we're inside a
-    // possibly long process of updating m_strings.
-    CRITICAL_SECTION m_csUpdate;
-
-    // If completer pointer is non-NULL, we must use it by calling its
-    // GetCompletions() method with the specified prefix to update the list of
-    // strings we iterate over when our Next() is called the next time.
     wxTextCompleter *m_completer;
+
+
+    // Critical section m_prefix and m_restart. It should be only entered for
+    // short periods of time, i.e. we shouldn't call any wxTextCompleter
+    // methods from inside, to prevent the main thread from blocking on it in
+    // UpdatePrefix().
+    CRITICAL_SECTION m_csRestart;
+
+    // If m_restart is true, we need to call wxTextCompleter::Start() with the
+    // given prefix to restart generating the completions.
     wxString m_prefix;
 
+    // Notice that we use LONG and not bool here to ensure that reading this
+    // value is atomic (32 bit reads are atomic operations under all Windows
+    // versions but reading bool isn't necessarily).
+    LONG m_restart;
 
-    wxDECLARE_NO_ASSIGN_CLASS(wxIEnumString);
+
+    wxDECLARE_NO_COPY_CLASS(wxIEnumString);
 };
 
 BEGIN_IID_TABLE(wxIEnumString)
@@ -324,8 +325,8 @@ END_IID_TABLE;
 IMPLEMENT_IUNKNOWN_METHODS(wxIEnumString)
 
 
-// This class gathers the auto-complete-related we use. It is allocated on
+// This class gathers the all auto-complete-related stuff we use. It is
-// demand by wxTextEntry when AutoComplete() is called.
+// allocated on demand by wxTextEntry when AutoComplete() is called.
 class wxTextAutoCompleteData wxBIND_OR_CONNECT_HACK_ONLY_BASE_CLASS
 {
 public:
@@ -337,6 +338,8 @@ public:
         m_autoComplete = NULL;
         m_autoCompleteDropDown = NULL;
         m_enumStrings = NULL;
+
+        m_fixedCompleter = NULL;
         m_customCompleter = NULL;
 
         m_connectedCharEvent = false;
@@ -409,6 +412,7 @@ public:
     ~wxTextAutoCompleteData()
     {
         delete m_customCompleter;
+        delete m_fixedCompleter;
 
         if ( m_enumStrings )
             m_enumStrings->Release();
@@ -427,7 +431,12 @@ public:
 
     void ChangeStrings(const wxArrayString& strings)
     {
-        m_enumStrings->ChangeStrings(strings);
+        if ( !m_fixedCompleter )
+            m_fixedCompleter = new wxTextCompleterFixed;
+
+        m_fixedCompleter->SetCompletions(strings);
+
+        m_enumStrings->ChangeCompleter(m_fixedCompleter);
 
         DoRefresh();
     }
@@ -435,6 +444,10 @@ public:
     // Takes ownership of the pointer if it is non-NULL.
     bool ChangeCustomCompleter(wxTextCompleter *completer)
     {
+        // Ensure that the old completer is not used any more before deleting
+        // it.
+        m_enumStrings->ChangeCompleter(completer);
+
         delete m_customCompleter;
         m_customCompleter = completer;
 
@@ -477,15 +490,33 @@ public:
         // to effectively disable auto-completion just fine. We could (and
         // probably should) use IAutoComplete::Enable(FALSE) for this too but
         // then we'd need to call Enable(TRUE) to turn it on back again later.
-
+        ChangeStrings(wxArrayString());
-        m_enumStrings->ChangeStrings(wxArrayString());
-
-        ChangeCustomCompleter(NULL);
     }
 
 private:
-    // Must be called after changing wxIEnumString::m_strings to really make
+    // Trivial wxTextCompleter implementation which always returns the same
-    // the changes stick.
+    // fixed array of completions.
+    class wxTextCompleterFixed : public wxTextCompleterSimple
+    {
+    public:
+        void SetCompletions(const wxArrayString& strings)
+        {
+            m_strings = strings;
+        }
+
+        virtual void GetCompletions(const wxString& WXUNUSED(prefix),
+                                    wxArrayString& res)
+        {
+            res = m_strings;
+        }
+
+    private:
+        wxArrayString m_strings;
+    };
+
+
+    // Must be called after changing the values to be returned by wxIEnumString
+    // to really make the changes stick.
     void DoRefresh()
     {
         m_enumStrings->Reset();
@@ -516,7 +547,7 @@ private:
 
         const wxString prefix = m_entry->GetRange(0, from);
 
-        m_enumStrings->UpdateStringsFromCompleter(m_customCompleter, prefix);
+        m_enumStrings->UpdatePrefix(prefix);
 
         DoRefresh();
     }
@@ -548,6 +579,9 @@ private:
     // Enumerator for strings currently used for auto-completion.
     wxIEnumString *m_enumStrings;
 
+    // Fixed string completer or NULL if none.
+    wxTextCompleterFixed *m_fixedCompleter;
+
     // Custom completer or NULL if none.
     wxTextCompleter *m_customCompleter;