2007/10/13, Youness Alaoui <[EMAIL PROTECTED]>:
>
> Hi,
> Thanks Square.
> Now time for my usual comments ;)
>
> 1 - Try to always send a clean patch, don't mix up two things in one
> patch, it just becomes difficult to know what to look at. [cut...]
Ok. I knew that i should always send an only patch... event if it's a big
patch with various things... Anyway ok, there's no problem.
2 - You put :
> > + if {[winfo exists ".messagebox_$unique"]} { return
> }
>
> This means that the customMessageBox will return "" what happens in that
> case? did you do a grep on all customMessageBox calls to see what
> would happen if that function returns nothing ? think of something like
> that :
> set ret [customMessageBox ... ]
> if { $ret == "yes" } { set close 1 } elseif {$ret == "no" } { set close 0
> }
> if { $close } { ... }
>
> With your patch, we'll get an error with "close : undefined variable"
> So you have two choices (or 3) :
> a) Make customMessageBox return "duplicate" (or "") and do a grep and
> wherever you find a call to customMessageBox, you make sure to check for
> the "duplicate" case (and make sure that if you just return that it won't
> affect the function that called the function that called
> customMessageBox, etc...)
> b) return "cancel" and make sure that it won't affect anything...
Ok. But for the moment only ContainerClose can open more windows... so i
think that the "duplicate" case should go only in that proc. Anyway i prefer
"duplicate" than "cancel".
What i can do now is to check if every procs that call customMessageBox have
the correct check about the answer, I mean:
(the "wrong" case): If {$answer == "yes"} {...} else {...}
Instead of: if {$answer == "yes"} {...} elseif {$answer == "no"} {...}
etc...
3 - Why did you name the variable "block" you could have named it "unique"
> instead, it would have been more logical and easier to
> understand/maintain
You are right... It was a "temporary name" to test the proc and I forgot to
change it :P
Thanks,
> KaKaRoTo
>
Thanks to you for explanation.
About the patches i'll send them as soon as possible. Atm i am working also
on others things.
Bye.
Square87
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
Amsn-devel mailing list
Amsn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/amsn-devel
