[PATCH v3 1/6] shell32/autocomplete: Fill and display the auto-suggest listbox in a separate function
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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)
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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
Signed-off-by: Huw Davies huw@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@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);
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@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 needlesscopies, 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@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@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);
This is needed for auto-append only AutoComplete controls.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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; }
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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);
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@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)
-
Gabriel Ivăncescu -
Huw Davies -
Nikolay Sivov