On Thu, Nov 1, 2018 at 3:09 PM Huw Davies <huw(a)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(a)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?).