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

Gabriel Ivăncescu gabrielopcode at gmail.com
Thu Nov 1 10:04:42 CDT 2018


On Thu, Nov 1, 2018 at 3:09 PM Huw Davies <huw at codeweavers.com> wrote:
>
> On Thu, Nov 01, 2018 at 02:23:50PM +0200, Gabriel Ivăncescu wrote:
> > On Thu, Nov 1, 2018 at 11:57 AM Huw Davies <huw at codeweavers.com> wrote:
> > > There's still a lot of code shuffling in this patch in
> > > dispatch_matching_strs().  Can you try to reduce that by more patch
> > > splitting?  At the moment there's just too much going on to clearly
> > > see what this patch is doing.
> > >
> > I'm afraid not. Try to look at the whole function to get an idea why
> > it's done this way. Maybe git's diff made it seem like there's more
> > changes than a total rewrite of the function, but it's actually a
> > rewrite.
>
> Well obviously I am looking at the whole function...
>
> Perhaps it'll look better once the 'search for last' bit is moved out.
> Avoiding variable names like a and b will help too.  'cnt' could be
> removed while you're at it.
>
> Huw.

Yeah a and b are removed, but I don't know about cnt since it's needed
for show_listbox, and i is incremented before that when adding the
items to the listbox. So the ability to calculate the count is lost by
that point. Sure I can save the original 'i' but that's no different
than just setting cnt directly.

Is cnt really a problem though? I thought it makes the code a bit more
descriptive in this case; i.e. makes it known exactly what i and a are
supposed to be (well now I named them i and k, is that alright?).



More information about the wine-devel mailing list