[PATCH v3 1/6] shell32/autocomplete: Fill and display the auto-suggest listbox in a separate function
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- v3: Split the patches. dlls/shell32/autocomplete.c | 77 ++++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index dfe7638..8cf0243 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -293,40 +293,11 @@ static void autoappend_str(IAutoCompleteImpl *ac, WCHAR *text, UINT len, WCHAR * heap_free(tmp); } -static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_flag flag) +static BOOL display_matching_strs(IAutoCompleteImpl *ac, WCHAR *text, UINT len, + HWND hwnd, enum autoappend_flag flag) { - HRESULT hr; - WCHAR *text; - UINT cpt, size, len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); - - if (flag != autoappend_flag_displayempty && len == 0) - { - if (ac->options & ACO_AUTOSUGGEST) - hide_listbox(ac, ac->hwndListBox); - return; - } - - size = len + 1; - if (!(text = heap_alloc(size * sizeof(WCHAR)))) - return; - len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)text); - if (len + 1 != size) - text = heap_realloc(text, (len + 1) * sizeof(WCHAR)); - - /* Reset it here to simplify the logic in aclist_expand for - empty strings, since it tracks changes using txtbackup, - and Reset needs to be called before IACList::Expand */ - IEnumString_Reset(ac->enumstr); - if (ac->aclist) - { - aclist_expand(ac, text); - if (text[len - 1] == '\\' || text[len - 1] == '/') - flag = autoappend_flag_no; - } - - /* Set txtbackup to point to text itself (which must not be released) */ - heap_free(ac->txtbackup); - ac->txtbackup = text; + /* Return FALSE if we need to hide the listbox */ + UINT cpt; if (ac->options & ACO_AUTOSUGGEST) { @@ -335,6 +306,7 @@ static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_ } for (cpt = 0;;) { + HRESULT hr; LPOLESTR strs = NULL; ULONG fetched; @@ -379,8 +351,47 @@ static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_ SendMessageW(ac->hwndListBox, WM_SETREDRAW, TRUE, 0); } else + return FALSE; + } + return TRUE; +} + +static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_flag flag) +{ + WCHAR *text; + UINT size, len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + + if (flag != autoappend_flag_displayempty && len == 0) + { + if (ac->options & ACO_AUTOSUGGEST) hide_listbox(ac, ac->hwndListBox); + return; } + + size = len + 1; + if (!(text = heap_alloc(size * sizeof(WCHAR)))) + return; + len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)text); + if (len + 1 != size) + text = heap_realloc(text, (len + 1) * sizeof(WCHAR)); + + /* Reset it here to simplify the logic in aclist_expand for + empty strings, since it tracks changes using txtbackup, + and Reset needs to be called before IACList::Expand */ + IEnumString_Reset(ac->enumstr); + if (ac->aclist) + { + aclist_expand(ac, text); + if (text[len - 1] == '\\' || text[len - 1] == '/') + flag = autoappend_flag_no; + } + + /* Set txtbackup to point to text itself (which must not be released) */ + heap_free(ac->txtbackup); + ac->txtbackup = text; + + if (!display_matching_strs(ac, text, len, hwnd, flag)) + hide_listbox(ac, ac->hwndListBox); } static void destroy_autocomplete_object(IAutoCompleteImpl *ac) -- 1.9.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 8cf0243..b3f86f3 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -109,6 +109,21 @@ static void hide_listbox(IAutoCompleteImpl *ac, HWND hwnd) SendMessageW(hwnd, LB_RESETCONTENT, 0, 0); } +static void show_listbox(IAutoCompleteImpl *ac, UINT cnt) +{ + RECT r; + UINT width, height; + + GetWindowRect(ac->hwndEdit, &r); + SendMessageW(ac->hwndListBox, LB_CARETOFF, 0, 0); + + /* Windows XP displays 7 lines at most, then it uses a scroll bar */ + height = SendMessageW(ac->hwndListBox, LB_GETITEMHEIGHT, 0, 0) * min(cnt + 1, 7); + width = r.right - r.left; + + SetWindowPos(ac->hwndListBox, HWND_TOP, r.left, r.bottom + 1, width, height, SWP_SHOWWINDOW); +} + static size_t format_quick_complete(WCHAR *dst, const WCHAR *qc, const WCHAR *str, size_t str_len) { /* Replace the first %s directly without using snprintf, to avoid @@ -339,15 +354,7 @@ static BOOL display_matching_strs(IAutoCompleteImpl *ac, WCHAR *text, UINT len, { if (cpt) { - RECT r; - UINT height = SendMessageW(ac->hwndListBox, LB_GETITEMHEIGHT, 0, 0); - SendMessageW(ac->hwndListBox, LB_CARETOFF, 0, 0); - GetWindowRect(hwnd, &r); - /* It seems that Windows XP displays 7 lines at most - and otherwise displays a vertical scroll bar */ - SetWindowPos(ac->hwndListBox, HWND_TOP, - r.left, r.bottom + 1, r.right - r.left, height * min(cpt + 1, 7), - SWP_SHOWWINDOW ); + show_listbox(ac, cpt); SendMessageW(ac->hwndListBox, WM_SETREDRAW, TRUE, 0); } else -- 1.9.1
Signed-off-by: Huw Davies <huw(a)codeweavers.com>
Windows doesn't reset and re-enumerate it everytime autocompletion happens, and it also sorts the strings. This matches it more closely and makes it more useable on large lists as well. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 207 +++++++++++++++++++++++++++++++++----------- 1 file changed, 157 insertions(+), 50 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index b3f86f3..9cfce4b 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -25,7 +25,6 @@ - implement ACO_FILTERPREFIXES style - implement ACO_RTLREADING style - implement ResetEnumerator - - string compares should be case-insensitive, the content of the list should be sorted */ #include "config.h" @@ -62,6 +61,8 @@ typedef struct LONG ref; BOOL initialized; BOOL enabled; + UINT enum_strs_num; + WCHAR **enum_strs; HWND hwndEdit; HWND hwndListBox; WNDPROC wpOrigEditProc; @@ -103,10 +104,102 @@ static void set_text_and_selection(IAutoCompleteImpl *ac, HWND hwnd, WCHAR *text CallWindowProcW(proc, hwnd, EM_SETSEL, start, end); } +static int enumerate_strings_cmpfn(const void *a, const void *b) +{ + return strcmpiW(*(WCHAR* const*)a, *(WCHAR* const*)b); +} + +static void enumerate_strings(IAutoCompleteImpl *ac) +{ + /* + Enumerate all of the strings and sort them in the internal list. + + We don't free the enumerated strings (except on error) to avoid needless + copies, until the next reset (or the object itself is destroyed) + */ + UINT i, cur, array_size = 1024, curblock_size = array_size, numstrs = 0; + LPOLESTR *strs = NULL, *tmp; + + for (;;) + { + LONG rem; + BOOL break_enum = FALSE; + + if ((tmp = heap_realloc(strs, array_size * sizeof(*strs))) == NULL) + goto fail; + strs = tmp; + rem = curblock_size; + + while (rem > 0) + { + ULONG n = 0; + cur = array_size - rem; + IEnumString_Next(ac->enumstr, rem, &strs[cur], &n); + if (n == 0) + { + break_enum = TRUE; + break; + } + rem -= n; + } + if (break_enum) break; + curblock_size = array_size; + array_size += curblock_size; + } + + /* Allocate even if there were zero strings enumerated, to mark it non-NULL */ + numstrs = cur; + if ((tmp = heap_realloc(strs, numstrs * sizeof(*strs)))) + { + strs = tmp; + if (numstrs > 0) + qsort(strs, numstrs, sizeof(*strs), enumerate_strings_cmpfn); + + ac->enum_strs = strs; + ac->enum_strs_num = numstrs; + return; + } + +fail: + i = numstrs; + while (i--) + CoTaskMemFree(strs[i]); + heap_free(strs); +} + +static UINT find_first_matching_enum_str(IAutoCompleteImpl *ac, WCHAR *text, UINT len) +{ + WCHAR **strs = ac->enum_strs; + UINT index = ~0, a = 0, b = ac->enum_strs_num; + while (a < b) + { + UINT i = (a + b - 1) / 2; + int cmp = strncmpiW(text, strs[i], len); + if (cmp == 0) index = i; + if (cmp <= 0) b = i; + else a = i + 1; + } + return index; +} + +static void free_enum_strs(IAutoCompleteImpl *ac) +{ + WCHAR **strs = ac->enum_strs; + if (strs) + { + UINT i = ac->enum_strs_num; + ac->enum_strs = NULL; + while (i--) + CoTaskMemFree(strs[i]); + heap_free(strs); + } +} + static void hide_listbox(IAutoCompleteImpl *ac, HWND hwnd) { ShowWindow(hwnd, SW_HIDE); SendMessageW(hwnd, LB_RESETCONTENT, 0, 0); + free_enum_strs(ac); } static void show_listbox(IAutoCompleteImpl *ac, UINT cnt) @@ -240,7 +333,11 @@ static LRESULT change_selection(IAutoCompleteImpl *ac, HWND hwnd, UINT key) static BOOL do_aclist_expand(IAutoCompleteImpl *ac, WCHAR *txt, WCHAR *last_delim) { - WCHAR c = last_delim[1]; + WCHAR c; + free_enum_strs(ac); + IEnumString_Reset(ac->enumstr); /* call before expand */ + + c = last_delim[1]; last_delim[1] = '\0'; IACList_Expand(ac->aclist, txt); last_delim[1] = c; @@ -276,6 +373,9 @@ static BOOL aclist_expand(IAutoCompleteImpl *ac, WCHAR *txt) while (i--) if (strchrW(delims, txt[i])) return do_aclist_expand(ac, txt, &txt[i]); + + /* Windows doesn't expand without a delim, but it does reset */ + free_enum_strs(ac); } return FALSE; @@ -312,60 +412,60 @@ static BOOL display_matching_strs(IAutoCompleteImpl *ac, WCHAR *text, UINT len, HWND hwnd, enum autoappend_flag flag) { /* Return FALSE if we need to hide the listbox */ - UINT cpt; + WCHAR **str = ac->enum_strs; + UINT cnt, a, b, i; + if (!str) return (ac->options & ACO_AUTOSUGGEST) ? FALSE : TRUE; - if (ac->options & ACO_AUTOSUGGEST) + if (len) { - SendMessageW(ac->hwndListBox, WM_SETREDRAW, FALSE, 0); - SendMessageW(ac->hwndListBox, LB_RESETCONTENT, 0, 0); - } - for (cpt = 0;;) - { - HRESULT hr; - LPOLESTR strs = NULL; - ULONG fetched; - - hr = IEnumString_Next(ac->enumstr, 1, &strs, &fetched); - if (hr != S_OK) - break; - - if (!strncmpiW(text, strs, len)) + i = find_first_matching_enum_str(ac, text, len); + if (i == ~0) + return (ac->options & ACO_AUTOSUGGEST) ? FALSE : TRUE; + + if (flag == autoappend_flag_yes) + autoappend_str(ac, text, len, str[i], hwnd); + if (!(ac->options & ACO_AUTOSUGGEST)) + return TRUE; + + /* Find the last string that begins with text, starting the search from i, + which is guaranteed to find at least one string (if none other, then i) */ + a = i, b = ac->enum_strs_num; + do { - if (cpt == 0 && flag == autoappend_flag_yes) - { - autoappend_str(ac, text, len, strs, hwnd); - if (!(ac->options & ACO_AUTOSUGGEST)) - { - CoTaskMemFree(strs); - break; - } - } - - if (ac->options & ACO_AUTOSUGGEST) - SendMessageW(ac->hwndListBox, LB_ADDSTRING, 0, (LPARAM)strs); - - cpt++; - } - - CoTaskMemFree(strs); + UINT mid = (a + b - 1) / 2; + if (!strncmpiW(text, str[mid], len)) + a = mid + 1; + else + b = mid; + } while (a < b); } - - if (ac->options & ACO_AUTOSUGGEST) + else { - if (cpt) - { - show_listbox(ac, cpt); - SendMessageW(ac->hwndListBox, WM_SETREDRAW, TRUE, 0); - } - else + if (!(ac->options & ACO_AUTOSUGGEST)) + return TRUE; + i = 0; + a = ac->enum_strs_num; + if (a == 0) return FALSE; } + cnt = a - i; + + SendMessageW(ac->hwndListBox, WM_SETREDRAW, FALSE, 0); + SendMessageW(ac->hwndListBox, LB_RESETCONTENT, 0, 0); + SendMessageW(ac->hwndListBox, LB_INITSTORAGE, cnt, 0); + do + SendMessageW(ac->hwndListBox, LB_INSERTSTRING, -1, (LPARAM)str[i]); + while (++i < a); + + show_listbox(ac, cnt); + SendMessageW(ac->hwndListBox, WM_SETREDRAW, TRUE, 0); return TRUE; } static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_flag flag) { WCHAR *text; + BOOL expanded = FALSE; UINT size, len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); if (flag != autoappend_flag_displayempty && len == 0) @@ -382,28 +482,35 @@ static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_ if (len + 1 != size) text = heap_realloc(text, (len + 1) * sizeof(WCHAR)); - /* Reset it here to simplify the logic in aclist_expand for - empty strings, since it tracks changes using txtbackup, - and Reset needs to be called before IACList::Expand */ - IEnumString_Reset(ac->enumstr); if (ac->aclist) { - aclist_expand(ac, text); if (text[len - 1] == '\\' || text[len - 1] == '/') flag = autoappend_flag_no; + expanded = aclist_expand(ac, text); + } + if (expanded || !ac->enum_strs) + { + if (!expanded) IEnumString_Reset(ac->enumstr); + enumerate_strings(ac); } - /* Set txtbackup to point to text itself (which must not be released) */ + /* Set txtbackup to point to text itself (which must not be released), + and it must be done here since aclist_expand uses it to track changes */ heap_free(ac->txtbackup); ac->txtbackup = text; if (!display_matching_strs(ac, text, len, hwnd, flag)) - hide_listbox(ac, ac->hwndListBox); + { + /* Hide the listbox, but do not clear the enum strs, to match Windows */ + ShowWindow(ac->hwndListBox, SW_HIDE); + SendMessageW(ac->hwndListBox, LB_RESETCONTENT, 0, 0); + } } static void destroy_autocomplete_object(IAutoCompleteImpl *ac) { ac->hwndEdit = NULL; + free_enum_strs(ac); if (ac->hwndListBox) DestroyWindow(ac->hwndListBox); IAutoComplete2_Release(&ac->IAutoComplete2_iface); -- 1.9.1
On Thu, Oct 25, 2018 at 09:04:52PM +0300, Gabriel Ivăncescu wrote:
Windows doesn't reset and re-enumerate it everytime autocompletion happens, and it also sorts the strings. This matches it more closely and makes it more useable on large lists as well.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 207 +++++++++++++++++++++++++++++++++----------- 1 file changed, 157 insertions(+), 50 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index b3f86f3..9cfce4b 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -25,7 +25,6 @@ - implement ACO_FILTERPREFIXES style - implement ACO_RTLREADING style - implement ResetEnumerator - - string compares should be case-insensitive, the content of the list should be sorted
*/ #include "config.h" @@ -62,6 +61,8 @@ typedef struct LONG ref; BOOL initialized; BOOL enabled; + UINT enum_strs_num; + WCHAR **enum_strs; HWND hwndEdit; HWND hwndListBox; WNDPROC wpOrigEditProc; @@ -103,10 +104,102 @@ static void set_text_and_selection(IAutoCompleteImpl *ac, HWND hwnd, WCHAR *text CallWindowProcW(proc, hwnd, EM_SETSEL, start, end); }
+static int enumerate_strings_cmpfn(const void *a, const void *b) +{ + return strcmpiW(*(WCHAR* const*)a, *(WCHAR* const*)b); +} + +static void enumerate_strings(IAutoCompleteImpl *ac) +{ + /* + Enumerate all of the strings and sort them in the internal list. + + We don't free the enumerated strings (except on error) to avoid needless + copies, until the next reset (or the object itself is destroyed) + */ + UINT i, cur, array_size = 1024, curblock_size = array_size, numstrs = 0; + LPOLESTR *strs = NULL, *tmp; + + for (;;) + { + LONG rem; + BOOL break_enum = FALSE; + + if ((tmp = heap_realloc(strs, array_size * sizeof(*strs))) == NULL) + goto fail; + strs = tmp; + rem = curblock_size; + + while (rem > 0) + { + ULONG n = 0; + cur = array_size - rem; + IEnumString_Next(ac->enumstr, rem, &strs[cur], &n); + if (n == 0) + { + break_enum = TRUE; + break; + } + rem -= n; + } + if (break_enum) break; + curblock_size = array_size; + array_size += curblock_size; + } + + /* Allocate even if there were zero strings enumerated, to mark it non-NULL */ + numstrs = cur;
There are too many variables tracking size in this block which makes the whole thing confusing. You should just need array_size, cur and n. If you really need a boolean to break out of the outer loop then name that variable 'done' and change the for (;;) -> while (!done) Huw.
On Tue, Oct 30, 2018 at 12:15 PM Huw Davies <huw(a)codeweavers.com> wrote:
There are too many variables tracking size in this block which makes the whole thing confusing. You should just need array_size, cur and n. If you really need a boolean to break out of the outer loop then name that variable 'done' and change the for (;;) -> while (!done)
Huw.
Yeah I can cull some variables. My reasoning for why I chose curblock_size was to make it more flexible so that the "rate of expansion" can be easily adjusted, should it be needed. If I make it like array_size *= 2, then I'll have to hardcode for this when calculating rem (or cur), unlike now where changing the rate of expansion can be easily done by adjusting just the curblock_size = array_size line (to literally any number, except zero). I'll change it though, no problem.
BTW about the boolean, I just want to ask to be sure about this please: The alternative would be to use a small goto as it was originally, all it does is basically break twice to outer scope (i.e. it's like "break 2" if C had such feature, or "break label" in some other languages). I suppose that's not acceptable though, right? I'm asking because a lot of other code-bases are ok with this use of goto (to break out of nested scope to outer scope), so I'm a bit "used" to it but obviously I'll go with what you have to say.
On Tue, Oct 30, 2018 at 01:19:04PM +0200, Gabriel Ivăncescu wrote:
BTW about the boolean, I just want to ask to be sure about this please:
The alternative would be to use a small goto as it was originally, all it does is basically break twice to outer scope (i.e. it's like "break 2" if C had such feature, or "break label" in some other languages). I suppose that's not acceptable though, right?
You already know the answer to the goto question. The reason I was querying about the need for the boolean, is that it's essentially mirroring n == 0 so, at first look, it might be possible to rewrite using that condition on the outer loop. Huw.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 9cfce4b..5cf990d 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -622,10 +622,15 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, hide_listbox(This, This->hwndListBox); return 0; case WM_KILLFOCUS: - if ((This->options & ACO_AUTOSUGGEST) && ((HWND)wParam != This->hwndListBox)) + if (This->options & ACO_AUTOSUGGEST) { - hide_listbox(This, This->hwndListBox); + if ((HWND)wParam == This->hwndListBox) break; + ShowWindow(This->hwndListBox, SW_HIDE); + SendMessageW(This->hwndListBox, LB_RESETCONTENT, 0, 0); } + + /* Reset the enumerator if it's not visible anymore */ + if (!IsWindowVisible(hwnd)) free_enum_strs(This); break; case WM_KEYDOWN: return ACEditSubclassProc_KeyDown(This, hwnd, uMsg, wParam, lParam); -- 1.9.1
This is needed for auto-append only AutoComplete controls. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/autocomplete.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 5cf990d..67984da 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -472,6 +472,8 @@ static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_ { if (ac->options & ACO_AUTOSUGGEST) hide_listbox(ac, ac->hwndListBox); + else + free_enum_strs(ac); return; } -- 1.9.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/shell32/tests/autocomplete.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/dlls/shell32/tests/autocomplete.c b/dlls/shell32/tests/autocomplete.c index 66bd472..8265bf9 100644 --- a/dlls/shell32/tests/autocomplete.c +++ b/dlls/shell32/tests/autocomplete.c @@ -274,6 +274,7 @@ struct string_enumerator WCHAR **data; int data_len; int cur; + UINT num_resets; UINT num_expand; WCHAR last_expand[32]; }; @@ -349,6 +350,7 @@ static HRESULT WINAPI string_enumerator_Reset(IEnumString *iface) struct string_enumerator *this = impl_from_IEnumString(iface); this->cur = 0; + this->num_resets++; return S_OK; } @@ -456,6 +458,7 @@ static void test_aclist_expand(HWND hwnd_edit, void *enumerator) static WCHAR str2[] = {'t','e','s','t','\\','f','o','o','\\','b','a','r','\\','b','a',0}; static WCHAR str2a[] = {'t','e','s','t','\\','f','o','o','\\','b','a','r','\\',0}; static WCHAR str2b[] = {'t','e','s','t','\\','f','o','o','\\','b','a','r','\\','b','a','z','_','b','b','q','\\',0}; + obj->num_resets = 0; ok(obj->num_expand == 0, "Expected 0 expansions, got %u\n", obj->num_expand); SendMessageW(hwnd_edit, WM_SETTEXT, 0, (LPARAM)str1); @@ -464,12 +467,14 @@ static void test_aclist_expand(HWND hwnd_edit, void *enumerator) dispatch_messages(); ok(obj->num_expand == 1, "Expected 1 expansion, got %u\n", obj->num_expand); ok(lstrcmpW(obj->last_expand, str1a) == 0, "Expected %s, got %s\n", wine_dbgstr_w(str1a), wine_dbgstr_w(obj->last_expand)); + ok(obj->num_resets == 1, "Expected 1 reset, got %u\n", obj->num_resets); SendMessageW(hwnd_edit, WM_SETTEXT, 0, (LPARAM)str2); SendMessageW(hwnd_edit, EM_SETSEL, ARRAY_SIZE(str2) - 1, ARRAY_SIZE(str2) - 1); SendMessageW(hwnd_edit, WM_CHAR, 'z', 1); dispatch_messages(); ok(obj->num_expand == 2, "Expected 2 expansions, got %u\n", obj->num_expand); ok(lstrcmpW(obj->last_expand, str2a) == 0, "Expected %s, got %s\n", wine_dbgstr_w(str2a), wine_dbgstr_w(obj->last_expand)); + ok(obj->num_resets == 2, "Expected 2 resets, got %u\n", obj->num_resets); SetFocus(hwnd_edit); SendMessageW(hwnd_edit, WM_CHAR, '_', 1); SendMessageW(hwnd_edit, WM_CHAR, 'b', 1); @@ -479,20 +484,24 @@ static void test_aclist_expand(HWND hwnd_edit, void *enumerator) SendMessageW(hwnd_edit, WM_CHAR, 'q', 1); dispatch_messages(); ok(obj->num_expand == 2, "Expected 2 expansions, got %u\n", obj->num_expand); + ok(obj->num_resets == 2, "Expected 2 resets, got %u\n", obj->num_resets); SendMessageW(hwnd_edit, WM_CHAR, '\\', 1); dispatch_messages(); ok(obj->num_expand == 3, "Expected 3 expansions, got %u\n", obj->num_expand); ok(lstrcmpW(obj->last_expand, str2b) == 0, "Expected %s, got %s\n", wine_dbgstr_w(str2b), wine_dbgstr_w(obj->last_expand)); + ok(obj->num_resets == 3, "Expected 3 resets, got %u\n", obj->num_resets); SendMessageW(hwnd_edit, EM_SETSEL, ARRAY_SIZE(str1a) - 1, -1); SendMessageW(hwnd_edit, WM_CHAR, 'x', 1); SendMessageW(hwnd_edit, WM_CHAR, 'y', 1); dispatch_messages(); ok(obj->num_expand == 4, "Expected 4 expansions, got %u\n", obj->num_expand); ok(lstrcmpW(obj->last_expand, str1a) == 0, "Expected %s, got %s\n", wine_dbgstr_w(str1a), wine_dbgstr_w(obj->last_expand)); + ok(obj->num_resets == 4, "Expected 4 resets, got %u\n", obj->num_resets); SendMessageW(hwnd_edit, EM_SETSEL, ARRAY_SIZE(str1) - 1, -1); SendMessageW(hwnd_edit, WM_CHAR, 'x', 1); dispatch_messages(); ok(obj->num_expand == 4, "Expected 4 expansions, got %u\n", obj->num_expand); + ok(obj->num_resets == 5, "Expected 5 resets, got %u\n", obj->num_resets); } static void test_custom_source(void) @@ -502,6 +511,7 @@ static void test_custom_source(void) static WCHAR str_beta[] = {'a','u','t','o',' ','c','o','m','p','l','e','t','e',0}; static WCHAR str_au[] = {'a','u',0}; static WCHAR *suggestions[] = { str_alpha, str_alpha2, str_beta }; + struct string_enumerator *obj; IUnknown *enumerator; IAutoComplete2 *autocomplete; HWND hwnd_edit; @@ -516,6 +526,7 @@ static void test_custom_source(void) ok(hr == S_OK, "CoCreateInstance failed: %x\n", hr); string_enumerator_create((void**)&enumerator, suggestions, ARRAY_SIZE(suggestions)); + obj = (struct string_enumerator*)enumerator; hr = IAutoComplete2_SetOptions(autocomplete, ACO_AUTOSUGGEST | ACO_AUTOAPPEND); ok(hr == S_OK, "IAutoComplete2_SetOptions failed: %x\n", hr); @@ -528,11 +539,14 @@ static void test_custom_source(void) dispatch_messages(); SendMessageW(hwnd_edit, WM_GETTEXT, ARRAY_SIZE(buffer), (LPARAM)buffer); ok(lstrcmpW(str_beta, buffer) == 0, "Expected %s, got %s\n", wine_dbgstr_w(str_beta), wine_dbgstr_w(buffer)); + ok(obj->num_resets == 1, "Expected 1 reset, got %u\n", obj->num_resets); SendMessageW(hwnd_edit, EM_SETSEL, 0, -1); SendMessageW(hwnd_edit, WM_CHAR, '\b', 1); dispatch_messages(); SendMessageW(hwnd_edit, WM_GETTEXT, ARRAY_SIZE(buffer), (LPARAM)buffer); ok(buffer[0] == '\0', "Expected empty string, got %s\n", wine_dbgstr_w(buffer)); + ok(obj->num_resets == 1, "Expected 1 reset, got %u\n", obj->num_resets); + obj->num_resets = 0; /* hijack the window procedure */ HijackerWndProc_prev = (WNDPROC)SetWindowLongPtrW(hwnd_edit, GWLP_WNDPROC, (LONG_PTR)HijackerWndProc); @@ -545,6 +559,7 @@ static void test_custom_source(void) dispatch_messages(); SendMessageW(hwnd_edit, WM_GETTEXT, ARRAY_SIZE(buffer), (LPARAM)buffer); ok(lstrcmpW(str_au, buffer) == 0, "Expected %s, got %s\n", wine_dbgstr_w(str_au), wine_dbgstr_w(buffer)); + ok(obj->num_resets == 1, "Expected 1 reset, got %u\n", obj->num_resets); SendMessageW(hwnd_edit, EM_SETSEL, 0, -1); SendMessageW(hwnd_edit, WM_CHAR, '\b', 1); dispatch_messages(); @@ -558,6 +573,7 @@ static void test_custom_source(void) dispatch_messages(); SendMessageW(hwnd_edit, WM_GETTEXT, ARRAY_SIZE(buffer), (LPARAM)buffer); ok(lstrcmpW(str_beta, buffer) == 0, "Expected %s, got %s\n", wine_dbgstr_w(str_beta), wine_dbgstr_w(buffer)); + ok(obj->num_resets == 2, "Expected 2 resets, got %u\n", obj->num_resets); /* end of hijacks */ test_aclist_expand(hwnd_edit, enumerator); -- 1.9.1
On 10/25/2018 09:04 PM, Gabriel Ivăncescu wrote:
+ WCHAR *text; + UINT size, len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + + if (flag != autoappend_flag_displayempty && len == 0) + { + if (ac->options & ACO_AUTOSUGGEST) hide_listbox(ac, ac->hwndListBox); + return; } + + size = len + 1; + if (!(text = heap_alloc(size * sizeof(WCHAR)))) + return; + len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)text); + if (len + 1 != size) + text = heap_realloc(text, (len + 1) * sizeof(WCHAR)); Was this tested with some inconsistent WM_GETTEXT implementation? Why do you need to realloc?
On Sat, Oct 27, 2018 at 12:47 PM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 10/25/2018 09:04 PM, Gabriel Ivăncescu wrote:
+ WCHAR *text; + UINT size, len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + + if (flag != autoappend_flag_displayempty && len == 0) + { + if (ac->options & ACO_AUTOSUGGEST) hide_listbox(ac, ac->hwndListBox); + return; } + + size = len + 1; + if (!(text = heap_alloc(size * sizeof(WCHAR)))) + return; + len = SendMessageW(hwnd, WM_GETTEXT, size, (LPARAM)text); + if (len + 1 != size) + text = heap_realloc(text, (len + 1) * sizeof(WCHAR)); Was this tested with some inconsistent WM_GETTEXT implementation? Why do you need to realloc?
When I wrote that (was awhile ago) IIRC I followed the APIs based on MSDN docs, where it said the result can be smaller under certain circumstances with ANSI<->Unicode conversion. I don't know if Wine suffers from that, however since the edit control can be subclassed by apps which maybe imitate MSDN's gimmicks, it's probably a safer bet anyway. Also note that this patch just moved that function as it was already there, so if that's not needed, it should be done in a different patch, if there's nothing else wrong with this series. (i.e. it's out of scope for this patch at least)
participants (3)
-
Gabriel Ivăncescu -
Huw Davies -
Nikolay Sivov