From: Vadim Zeitlin Date: Sat, 16 Apr 2011 17:27:34 +0000 (+0000) Subject: Split wxTextCompleter into a base class and wxTextCompleterSimple. X-Git-Url: https://git.saurik.com/wxWidgets.git/commitdiff_plain/85047589a993f5c5529e1dae284be8af84cd68ef 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 --- diff --git a/include/wx/textcompleter.h b/include/wx/textcompleter.h index 76aae00a0d..2e084c9e03 100644 --- a/include/wx/textcompleter.h +++ b/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_ + diff --git a/interface/wx/textcompleter.h b/interface/wx/textcompleter.h index 275716c00d..609895a746 100644 --- a/interface/wx/textcompleter.h +++ b/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 - if it needs to access any shared data. + (this is currently always the case under MSW) so the appropriate + synchronization mechanism should be used to protect the shared data. @param prefix The possibly empty prefix that the user had already entered. diff --git a/samples/widgets/widgets.cpp b/samples/widgets/widgets.cpp index c5f4700478..c858d3f753 100644 --- a/samples/widgets/widgets.cpp +++ b/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) diff --git a/src/common/textentrycmn.cpp b/src/common/textentrycmn.cpp index 66a0a6452e..5b08496492 100644 --- a/src/common/textentrycmn.cpp +++ b/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 diff --git a/src/msw/textentry.cpp b/src/msw/textentry.cpp index d0bde2fad6..4d5b2785b0 100644 --- a/src/msw/textentry.cpp +++ b/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 -// update them from the custom completer if one is used. +// class simply forwards to wxTextCompleter associated with it. // // 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) - : m_strings(other.m_strings), - m_index(other.m_index) + void ChangeCompleter(wxTextCompleter *completer) { - Init(); - } + // Indicate to Next() that it should bail out as soon as possible. + { + CSLock lock(m_csRestart); - void ChangeStrings(const wxArrayString& strings) - { - CSLock lock(m_csStrings); + m_restart = TRUE; + } - m_strings = strings; - m_index = 0; + // 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 + // the old pointer will be destroyed after we return). + CSLock lock(m_csCompleter); + + m_completer = completer; } - void UpdateStringsFromCompleter(wxTextCompleter *completer, - const wxString& prefix) + void UpdatePrefix(const wxString& prefix) { - CSLock lock(m_csUpdate); + CSLock lock(m_csRestart); - // 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; + // 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() ) + return S_FALSE; - for ( const unsigned count = m_strings.size(); celt--; ++m_index ) + while ( celt-- ) { - if ( m_index == count ) + // Stop iterating if we need to update completions anyhow. + if ( m_restart ) return S_FALSE; - const wxWX2WCbuf wcbuf = m_strings[m_index].wc_str(); + 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,13 +187,21 @@ public: virtual HRESULT STDMETHODCALLTYPE Skip(ULONG celt) { - CSLock lock(m_csStrings); + if ( !celt ) + return E_INVALIDARG; - m_index += celt; - if ( m_index > m_strings.size() ) - { - m_index = m_strings.size(); + CSLock lock(m_csCompleter); + + if ( !RestartIfNeeded() ) return S_FALSE; + + while ( celt-- ) + { + if ( m_restart ) + return S_FALSE; + + if ( m_completer->GetNext().empty() ) + return S_FALSE; } return S_OK; @@ -197,22 +209,9 @@ public: virtual HRESULT STDMETHODCALLTYPE Reset() { - { - CSLock lock(m_csUpdate); + CSLock lock(m_csRestart); - if ( m_completer ) - { - // 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; + m_restart = TRUE; return S_OK; } @@ -222,11 +221,13 @@ public: if ( !ppEnum ) return E_POINTER; - CSLock lock(m_csStrings); - - wxIEnumString *e = new wxIEnumString(*this); + CSLock lock(m_csCompleter); + 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_csUpdate); + ::DeleteCriticalSection(&m_csRestart); + ::DeleteCriticalSection(&m_csCompleter); } // Common part of all ctors. void Init() { - ::InitializeCriticalSection(&m_csUpdate); - ::InitializeCriticalSection(&m_csStrings); + ::InitializeCriticalSection(&m_csCompleter); + ::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() { - wxTextCompleter *completer; - wxString prefix; + bool rc = true; + for ( ;; ) { - CSLock lock(m_csUpdate); - - completer = m_completer; - if ( !completer ) - return; + wxString prefix; + LONG restart; + { + CSLock lock(m_csRestart); - prefix = m_prefix; - } // Unlock m_csUpdate to allow the main thread to call our - // UpdateStringsFromCompleter() without blocking while we are - // generating completions. + prefix = m_prefix; + restart = m_restart; - // Notice that m_csStrings is locked by our caller already so no need - // to enter it again. - m_index = 0; - m_strings.clear(); - completer->GetCompletions(prefix, m_strings); + m_restart = FALSE; + } // Release m_csRestart before calling Start() to avoid blocking + // the main thread in UpdatePrefix() during its execution. - { - CSLock lock(m_csUpdate); + if ( !restart ) + break; - if ( m_completer == completer && m_prefix == 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. + rc = m_completer->Start(prefix); } + + return rc; } - // Critical section protecting m_strings and m_index. - CRITICAL_SECTION m_csStrings; + // Critical section protecting m_completer itself. It must be entered when + // 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. - wxArrayString m_strings; - unsigned m_index; + // The completer we delegate to for the completions generation. It is never + // NULL after the initial ChangeCompleter() call. + wxTextCompleter *m_completer; - // 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 -// demand by wxTextEntry when AutoComplete() is called. +// This class gathers the all auto-complete-related stuff we use. It is +// 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. - - m_enumStrings->ChangeStrings(wxArrayString()); - - ChangeCustomCompleter(NULL); + ChangeStrings(wxArrayString()); } private: - // Must be called after changing wxIEnumString::m_strings to really make - // the changes stick. + // Trivial wxTextCompleter implementation which always returns the same + // 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;