On Sat, 01 Oct 2011 14:03:38 +0100 Martin <[email protected]> wrote:
> On 01/10/2011 09:05, Mattias Gaertner wrote: > > On Sat, 01 Oct 2011 08:39:42 +0100 > > Martin<[email protected]> wrote: > > > >> On 01/10/2011 07:53, Mattias Gaertner wrote: > >>> On Sat, 01 Oct 2011 03:04:56 +0200 > >>> Hans-Peter Diettrich<[email protected]> wrote: > >>> > >>>> I don't understand the logic behind TControl.Click: > >>>> > >>>> procedure TControl.Click; > >>>> begin > >>>> //DebugLn(['TControl.Click ',DbgSName(Self)]); > >>>> if (not (csDesigning in ComponentState)) and (ActionLink<> nil) > >>>> and > >>>> ((Action=nil) or (@FOnClick<> @Action.OnExecute) or > >>>> Assigned(FOnClick)) then > >>>> ActionLink.Execute(Self) > >>>> else > >>>> if Assigned(FOnClick) then > >>>> FOnClick(Self); > >>>> end; > >>>> > >>>> > >> Maybe I overlook something, but > >> @FOnClick is the address of the variable holding the method reference > >> (or nil) > >> so is @Action.OnExecute > >> > >> So when can they ever be equal? > > You are right. > ... > > I added a CompareMem. > >> Also is that supposed to be a shortcut, saving the call, or is that > >> indeed supposed to modify behaviour? > > Good question. > > I guess it would be enough to check (not (csDesigning in > > ComponentState)) and (ActionLink<> nil). > > > > Maybe the Delphi docs have an answer. > > > > further more: > > ActionLink.Execute is virtual > > So comparing with the OnExecute method pointer is at the very least > insufficient. > > If ActionLink.Execute is overridden, it could call/do anything. True. But it seems Delphi does the same. > I would suggest to drop that check entirely Why? Do you have a real case where it hurts? > SVN Blame shows that it's been there for a long time, and always had the > @FOnClick => so it never triggered in the past, and there never was a > bug, while it did not trigger. Well, this allows many conclusions. ;) Mattias -- _______________________________________________ Lazarus mailing list [email protected] http://lists.lazarus.freepascal.org/mailman/listinfo/lazarus
