[Geany-devel] interactive typeahead seach for the sidebar

[Thomas Martitz]

Enrico Tröger schrieb:
> On Sat, 05 Sep 2009 18:30:13 +0200, Colomban wrote:
>
>   
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Hi,
>>
>> 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).
>
>
> Regards,
> Enrico
>   

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 
at testing.

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 :)

Best regards.