[PATCH v5 2/7] shell32/autocomplete: Move the autocomplete processing to a separate function

[Huw Davies]

On Wed, Sep 12, 2018 at 10:42:16PM +0300, Gabriel Ivăncescu wrote:
> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> ---
>  dlls/shell32/autocomplete.c | 135 ++++++++++++++++++++++++--------------------
>  1 file changed, 73 insertions(+), 62 deletions(-)

This wasn't exactly what I was expecting.  What I'd hoped to see was
the entire WM_KEYUP handler moved to a helper function, not half of
it.  [FWIW, renaming variable and re-formatting the code in the new
helper is a good plan].

After you do that, I then suggest pulling the code that deals with the
displayall stuff into a sub-helper---this is most of the code that you
have in autocomplete_text().

Huw.

> diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c
> index 9f35ff7..63fdf3d 100644
> --- a/dlls/shell32/autocomplete.c
> +++ b/dlls/shell32/autocomplete.c
> @@ -119,6 +119,76 @@ static size_t format_quick_complete(WCHAR *dst, const WCHAR *qc, const WCHAR *st
>      return dst - base;
>  }
>  
> +static void autocomplete_text(IAutoCompleteImpl *ac, WCHAR *text, UINT len, HWND hwnd, BOOL displayall)
> +{
> +    HRESULT hr;
> +    UINT cpt;
> +
> +    SendMessageW(ac->hwndListBox, LB_RESETCONTENT, 0, 0);
> +
> +    /* Set txtbackup to point to text itself (which must not be released) */
> +    heap_free(ac->txtbackup);
> +    ac->txtbackup = text;
> +
> +    if (!displayall && !len)
> +        return;
> +
> +    IEnumString_Reset(ac->enumstr);
> +    for(cpt = 0;;)
> +    {
> +        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 && (ac->options & ACO_AUTOAPPEND))
> +            {
> +                WCHAR buffW[255];
> +
> +                strcpyW(buffW, text);
> +                strcatW(buffW, &strs[len]);
> +                SetWindowTextW(hwnd, buffW);
> +                SendMessageW(hwnd, EM_SETSEL, len, strlenW(strs));
> +                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)
> +    {
> +        if (cpt)
> +        {
> +            RECT r;
> +            UINT height = SendMessageW(ac->hwndListBox, LB_GETITEMHEIGHT, 0, 0);
> +            SendMessageW(ac->hwndListBox, LB_CARETOFF, 0, 0);
> +            GetWindowRect(hwnd, &r);
> +            SetParent(ac->hwndListBox, HWND_DESKTOP);
> +            /* 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, min(height * 7, height*(cpt+1)),
> +                         SWP_SHOWWINDOW );
> +        }
> +        else
> +            ShowWindow(ac->hwndListBox, SW_HIDE);
> +    }
> +}
> +
>  static void destroy_autocomplete_object(IAutoCompleteImpl *ac)
>  {
>      ac->hwndEdit = NULL;
> @@ -133,12 +203,9 @@ static void destroy_autocomplete_object(IAutoCompleteImpl *ac)
>  static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
>  {
>      IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW);
> -    HRESULT hr;
>      WCHAR *hwndText;
> -    UINT len, size, cpt;
> -    RECT r;
> +    UINT len, size;
>      BOOL displayall = FALSE;
> -    int height, sel;
>  
>      if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
>  
> @@ -201,7 +268,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
>                      } else {
>                          heap_free(hwndText);
>                          if (IsWindowVisible(This->hwndListBox)) {
> -                            int count;
> +                            int count, sel;
>  
>                              count = SendMessageW(This->hwndListBox, LB_GETCOUNT, 0, 0);
>                              /* Change the selection */
> @@ -245,63 +312,7 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
>              if (len + 1 != size)
>                  hwndText = heap_realloc(hwndText, (len + 1) * sizeof(WCHAR));
>  
> -            SendMessageW(This->hwndListBox, LB_RESETCONTENT, 0, 0);
> -
> -            /* Set txtbackup to point to hwndText itself (which must not be released) */
> -            heap_free(This->txtbackup);
> -            This->txtbackup = hwndText;
> -
> -            if (!displayall && !len)
> -                break;
> -
> -            IEnumString_Reset(This->enumstr);
> -            for(cpt = 0;;) {
> -                LPOLESTR strs = NULL;
> -                ULONG fetched;
> -
> -                hr = IEnumString_Next(This->enumstr, 1, &strs, &fetched);
> -                if (hr != S_OK)
> -                    break;
> -
> -                if (!strncmpiW(hwndText, strs, len)) {
> -                    if (cpt == 0 && (This->options & ACO_AUTOAPPEND)) {
> -                        WCHAR buffW[255];
> -
> -                        strcpyW(buffW, hwndText);
> -                        strcatW(buffW, &strs[len]);
> -                        SetWindowTextW(hwnd, buffW);
> -                        SendMessageW(hwnd, EM_SETSEL, len, strlenW(strs));
> -                        if (!(This->options & ACO_AUTOSUGGEST)) {
> -                            CoTaskMemFree(strs);
> -                            break;
> -                        }
> -                    }
> -
> -                    if (This->options & ACO_AUTOSUGGEST)
> -                        SendMessageW(This->hwndListBox, LB_ADDSTRING, 0, (LPARAM)strs);
> -
> -                    cpt++;
> -                }
> -
> -                CoTaskMemFree(strs);
> -            }
> -
> -            if (This->options & ACO_AUTOSUGGEST) {
> -                if (cpt) {
> -                    height = SendMessageW(This->hwndListBox, LB_GETITEMHEIGHT, 0, 0);
> -                    SendMessageW(This->hwndListBox, LB_CARETOFF, 0, 0);
> -                    GetWindowRect(hwnd, &r);
> -                    SetParent(This->hwndListBox, HWND_DESKTOP);
> -                    /* It seems that Windows XP displays 7 lines at most
> -                       and otherwise displays a vertical scroll bar */
> -                    SetWindowPos(This->hwndListBox, HWND_TOP,
> -                                 r.left, r.bottom + 1, r.right - r.left, min(height * 7, height*(cpt+1)),
> -                                 SWP_SHOWWINDOW );
> -                } else {
> -                    ShowWindow(This->hwndListBox, SW_HIDE);
> -                }
> -            }
> -
> +            autocomplete_text(This, hwndText, len, hwnd, displayall);
>              break;
>          case WM_DESTROY:
>          {
> -- 
> 1.9.1
> 
> 
>