[PATCH v4 07/10] shell32/autocomplete: Redesign the window proc to trigger on key presses instead of key release

[Gabriel Ivăncescu]

On Tue, Sep 11, 2018 at 8:22 PM, Huw Davies <huw at codeweavers.com> wrote:
>
> Specifically the gotos, but I've already commented on the high
> identation level in this function (I know that's not your fault).
>
> It seems to me that you want the WM_KEYDOWN handler to call the
> WM_CHAR handler.  At the risk of repeating myself, having the handlers
> as a helper functions would make this easier.
>
> Huw.

I honestly don't think the gotos are too bad (especially the VK_DELETE
one since it's also very close), and my reasoning for it is because
all the variables that could be affected are set right before the goto
(ret, displayall, noautoappend), so there's no state to memorize,
because every other variable is set before both switch statements
(including parameters).

However, that probably won't convince you, so I'll maybe end up having
to rewrite this, but would a larger patch be acceptable then? I mean,
to get it in a "workable" state it's pretty hard to split it up.

That said there are still some difficulties with functions instead of
goto, since the goto doesn't jump directly into WM_CHAR but a
sub-block depending on condition. It is easy to make most of WM_CHAR a
separate function, but not the original if blocks. For example,
VK_DELETE's wParam is actually not a "control character" in terms of
WM_CHAR's wParam, yet it needs to use that same code path (that's why
it jumps inside the if statement, to share the code). Maybe I can give
an extra bool argument to the helper function, to know whether to go
the "control character" route or not. Definitely not as elegant in my
opinion, but if it has to be done heh.

But yeah, I won't be able to do this without rewriting it...


BTW just wondering but what is exactly bad about the goto in this
situation? (just the first one or both?) Of course, I know it's not
ideal, but what makes it so bad I mean? I'm one of those who dislikes
littering code with BOOLs and have a lot of checks just to avoid some
gotos (which make code cleaner in that situation), so it's not obvious
to me due to that background, that's why I'd like to know :-)