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