[Geany-devel] interactive typeahead seach for the sidebar
thomas.martitz at xxxxx
Sun Sep 6 22:33:39 UTC 2009
Enrico Tröger schrieb:
> On Sat, 05 Sep 2009 18:30:13 +0200, Colomban wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>> Thomas Martitz a écrit :
>>> My patch was buggy. I don't know how I didn't notice it while
>>> testing. This should fix it http://pastie.org/606912
>> Nothing important, but I wonder why you test for (!reload) as you
>> already check for (reload)? The check at the right of a || switch will
>> be tested only if the left one is not true, then if the right check
>> happens, then reload is necessarily false, isn't it?
>> But it just seems strange and complicated to me, not a problem anyway.
> Same here.
> Before the patch we only checked for "if (reload)". The function
> documentation already says:
> if reload is set, the doc argument has to be non-NULL, if reload is not
> set, doc is NULL. So, the new, additional check is somewhat
> unnecessary. It could be argumented as an additional sanity check but
> generally, document_open_file_full() is an internal function and so if
> the above conditions of the input argumens are not fullfilled, I would
> consider this as a bug in the calling code (still sanity checks are
> good, no doubt).
Well, before my patch document_open_file_full() *returned* st
raightly after finding the file in the internal open file list (in the
!reload case). I added the goto to maintain that behavior without
duplicating the "grab focus" code. Now that my latest fix removed the
goto also, the original behavior should be still maintained, hence I
added the if () (it went that road always if (reload == true), and only
if (!reload) *and* it wasn't found in the internal document list (i.e.
it's still NULL).
I'm not sure if it's really a bug of the calling code, and I'm also not
sure why I got that if () wrong in the first place without noticing it
The culprit is probably that the function basically does a reload, even
if (doc != NULL). But that was the original behavor which I never
intended to change.
Btw, I'm thinking the goto saved a difficult to figure out/parse if ()
construct, but that may just be me :)
More information about the Devel