[PATCH v5 1/4] shell32/autocomplete: Implement a cache and sort the enumerated strings for proper behavior

[Huw Davies]

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 at 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.