[PATCH v5 1/4] shell32/autocomplete: Implement a cache and sort the enumerated strings for proper behavior
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 | 209 +++++++++++++++++++++++++++++++------------- 1 file changed, 148 insertions(+), 61 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index b3f86f3..d557a4a 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,94 @@ static void set_text_and_selection(IAutoCompleteImpl *ac, HWND hwnd, WCHAR *text CallWindowProcW(proc, hwnd, EM_SETSEL, start, end); }
-static void hide_listbox(IAutoCompleteImpl *ac, HWND hwnd) +static int enumerate_strings_cmpfn(const void *a, const void *b) +{ + return strcmpiW(*(WCHAR* const*)a, *(WCHAR* const*)b); +} + +/* + 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) +*/ +static void enumerate_strings(IAutoCompleteImpl *ac) +{ + UINT cur = 0, array_size = 1024; + LPOLESTR *strs = NULL, *tmp; + ULONG read; + + do + { + if ((tmp = heap_realloc(strs, array_size * sizeof(*strs))) == NULL) + goto fail; + strs = tmp; + + do + { + if (FAILED(IEnumString_Next(ac->enumstr, array_size - cur, &strs[cur], &read))) + read = 0; + } while (read != 0 && (cur += read) < array_size); + + array_size *= 2; + } while (read != 0); + + /* Allocate even if there were zero strings enumerated, to mark it non-NULL */ + if ((tmp = heap_realloc(strs, cur * sizeof(*strs)))) + { + strs = tmp; + if (cur > 0) + qsort(strs, cur, sizeof(*strs), enumerate_strings_cmpfn); + + ac->enum_strs = strs; + ac->enum_strs_num = cur; + return; + } + +fail: + while (cur--) + CoTaskMemFree(strs[cur]); + heap_free(strs); +} + +static UINT find_matching_enum_str(IAutoCompleteImpl *ac, UINT start, WCHAR *text, + UINT len, int direction) +{ + WCHAR **strs = ac->enum_strs; + UINT index = ~0, a = start, 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; + cmp = direction; + } + 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, BOOL reset) { ShowWindow(hwnd, SW_HIDE); SendMessageW(hwnd, LB_RESETCONTENT, 0, 0); + if (reset) free_enum_strs(ac); }
static void show_listbox(IAutoCompleteImpl *ac, UINT cnt) @@ -167,14 +252,14 @@ static BOOL select_item_with_return_key(IAutoCompleteImpl *ac, HWND hwnd) { len = SendMessageW(hwndListBox, LB_GETTEXT, sel, (LPARAM)text); set_text_and_selection(ac, hwnd, text, 0, len); - hide_listbox(ac, hwndListBox); + hide_listbox(ac, hwndListBox, TRUE); ac->no_fwd_char = '\r'; /* RETURN char */ heap_free(text); return TRUE; } } } - hide_listbox(ac, hwndListBox); + hide_listbox(ac, hwndListBox, TRUE); return FALSE; }
@@ -240,7 +325,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 +365,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,66 +404,58 @@ 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, i, k; + 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); + i = find_matching_enum_str(ac, 0, text, len, -1); + 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 index beyond the last string that matches */ + k = find_matching_enum_str(ac, i + 1, text, len, 1); + k = (k == ~0 ? i : k) + 1; } - 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)) - { - 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); - } - - 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; + k = ac->enum_strs_num; + if (k == 0) return FALSE; } + cnt = k - 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 < k); + + 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) { if (ac->options & ACO_AUTOSUGGEST) - hide_listbox(ac, ac->hwndListBox); + hide_listbox(ac, ac->hwndListBox, TRUE); return; }
@@ -382,28 +466,31 @@ 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_listbox(ac, ac->hwndListBox, FALSE); }
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); @@ -421,7 +508,7 @@ static LRESULT ACEditSubclassProc_KeyDown(IAutoCompleteImpl *ac, HWND hwnd, UINT /* When pressing ESC, Windows hides the auto-suggest listbox, if visible */ if ((ac->options & ACO_AUTOSUGGEST) && IsWindowVisible(ac->hwndListBox)) { - hide_listbox(ac, ac->hwndListBox); + hide_listbox(ac, ac->hwndListBox, FALSE); ac->no_fwd_char = 0x1B; /* ESC char */ return 0; } @@ -448,7 +535,7 @@ static LRESULT ACEditSubclassProc_KeyDown(IAutoCompleteImpl *ac, HWND hwnd, UINT }
if (ac->options & ACO_AUTOSUGGEST) - hide_listbox(ac, ac->hwndListBox); + hide_listbox(ac, ac->hwndListBox, TRUE); heap_free(text); return 0; } @@ -512,12 +599,12 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, { case CB_SHOWDROPDOWN: if (This->options & ACO_AUTOSUGGEST) - hide_listbox(This, This->hwndListBox); + hide_listbox(This, This->hwndListBox, TRUE); return 0; case WM_KILLFOCUS: if ((This->options & ACO_AUTOSUGGEST) && ((HWND)wParam != This->hwndListBox)) { - hide_listbox(This, This->hwndListBox); + hide_listbox(This, This->hwndListBox, FALSE); } break; case WM_KEYDOWN: @@ -583,7 +670,7 @@ static LRESULT APIENTRY ACLBoxSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, break; len = SendMessageW(hwnd, LB_GETTEXT, sel, (LPARAM)msg); set_text_and_selection(This, This->hwndEdit, msg, 0, len); - hide_listbox(This, hwnd); + hide_listbox(This, hwnd, TRUE); heap_free(msg); break; default: @@ -861,7 +948,7 @@ static HRESULT WINAPI IAutoComplete2_fnSetOptions( if ((This->options & ACO_AUTOSUGGEST) && This->hwndEdit && !This->hwndListBox) create_listbox(This); else if (!(This->options & ACO_AUTOSUGGEST) && This->hwndListBox) - hide_listbox(This, This->hwndListBox); + hide_listbox(This, This->hwndListBox, TRUE);
return hr; }
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/shell32/autocomplete.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index d557a4a..0770549 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -602,10 +602,14 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, hide_listbox(This, This->hwndListBox, TRUE); return 0; case WM_KILLFOCUS: - if ((This->options & ACO_AUTOSUGGEST) && ((HWND)wParam != This->hwndListBox)) + if (This->options & ACO_AUTOSUGGEST) { + if ((HWND)wParam == This->hwndListBox) break; hide_listbox(This, This->hwndListBox, FALSE); } + + /* 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 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 0770549..ab95a70 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -455,7 +455,8 @@ static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_ if (flag != autoappend_flag_displayempty && len == 0) { if (ac->options & ACO_AUTOSUGGEST) - hide_listbox(ac, ac->hwndListBox, TRUE); + hide_listbox(ac, ac->hwndListBox, FALSE); + free_enum_strs(ac); return; }
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
It's periodically set to 0 to prevent cascading failures.
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 Thu, Nov 01, 2018 at 09:41:53PM +0200, 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 | 209 +++++++++++++++++++++++++++++++------------- 1 file changed, 148 insertions(+), 61 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index b3f86f3..d557a4a 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -240,7 +325,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;
Missed this before, but this is an unnecessary change. Let's also have a blank line between variable declarations and code.
- 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 +365,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,66 +404,58 @@ 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, i, k;
- 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);
i = find_matching_enum_str(ac, 0, text, len, -1);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 index beyond the last string that matches */k = find_matching_enum_str(ac, i + 1, text, len, 1);k = (k == ~0 ? i : k) + 1;
- 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)){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);- }
- 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;k = ac->enum_strs_num;if (k == 0) return FALSE;- cnt = k - i;
I mentioned the variable names in this function last time. 'i' and 'k' would be ok for loops, but here they're not used in that way; 'start' and 'end' would be much better choices. And like I said, then you really don't need 'cnt' at all.
- 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 < k);
It's minor, but I would probably write this loop as a for loop with an empty initialization statement.
Huw.
On Fri, Nov 2, 2018 at 1:45 PM Huw Davies huw@codeweavers.com wrote:
Missed this before, but this is an unnecessary change. Let's also have a blank line between variable declarations and code.
Ok.
I mentioned the variable names in this function last time. 'i' and 'k' would be ok for loops, but here they're not used in that way; 'start' and 'end' would be much better choices. And like I said, then you really don't need 'cnt' at all.
But then I'd still need an 'i' to iterate through when inserting the strings, if I don't use cnt (since it's needed for show_listbox below). I won't be able to do end - start there if I change start...
So net effect isn't really better compared to cnt (same amount of variables; instead of cnt, start, end we'll need i, start, end), though renaming the vars sounds like a good idea.
On Fri, Nov 02, 2018 at 03:28:00PM +0200, Gabriel Ivăncescu wrote:
On Fri, Nov 2, 2018 at 1:45 PM Huw Davies huw@codeweavers.com wrote:
Missed this before, but this is an unnecessary change. Let's also have a blank line between variable declarations and code.
Ok.
I mentioned the variable names in this function last time. 'i' and 'k' would be ok for loops, but here they're not used in that way; 'start' and 'end' would be much better choices. And like I said, then you really don't need 'cnt' at all.
But then I'd still need an 'i' to iterate through when inserting the strings, if I don't use cnt (since it's needed for show_listbox below). I won't be able to do end - start there if I change start...
So net effect isn't really better compared to cnt (same amount of variables; instead of cnt, start, end we'll need i, start, end), though renaming the vars sounds like a good idea.
Right, sorry, I missed that you use cnt in show_listbox(). So yeah, rename i -> start, k -> end, then either use cnt and increment start in the loop, or use a i in the loop without cnt. I don't mind either way.
Huw.
-
Gabriel Ivăncescu -
Huw Davies