Re: [Vimprobable-users] [PATCH] Further refactoring

[Daniel C. <dan...@gm...>]

Hi Hannes!
> I think this is a good idea, but as you probably guessed (and noticed,
> seeing your followup patch), such a major change will also need major
> testing. I will get to it by using your version for my regular work,
> but it will take some time to gain overall confidence. Testing by
> others would be appreciated, too, of course!

If the refactored code needs major testing, we can also test some more patches
too:)

Today I spent some time to check vimprobable for memory leaks with valgrind
and found some none freed strings in conjunction with the values filled by
jsapi_evaluate_script. It isn't easy for me to under stand the code related to
the editor feature - which of the variables 'value' or 'message' contains a
newly allocated string and when can they be freed. At the moment I've put in
some possible missed g_free() calls, but I'm not sure if this was done right.

In generals wen should find a way to reduce the number of g_free calls in the
open_editor function, if this is possible.

During the hunting of memory leaky I found out, that the hint could not be
cleaned/removed if the incremental search feature was disabled. This problem
is fixed in patch 07-Fixed-wrong-placed-ifdef-ENABLE_INCREMENTAL_SEARCH.patch.

Like HP annotated in the code, there are still heavy memory leaks in complete
function. But this function is difficult to understand. I think we should
split the logic and have a function that build up a list of matching items
(for each completion type 'commands', 'settings', 'tags', 'url-history'),
and another function to visualize these items to reduce the complexity of this
part of code. In the way the complete function is implemented at the moment,
I can't see when the memory should be freed and when not.

Daniel