Re: [Gnustep-cvs] Commit Update

[Quentin Mathé]

Le 14 août 04, à 14:46, Fred Kiefer a écrit :

> > +2004-08-13  Quentin Mathe <address@hidden>
> > +        +      * Source/NSComboBoxCell.m:
> > +       * Source/NSComboBox.m:
> > +	Fixed broken combo box behavior introduced June 17 with version 
> > 1.27,
> > +	which produced wrong tracking in the button cell and extra mouse 
> > down
> > +       call in the NSComboBox superclass. Now the rewritten version is also
> > +	documented. +	Also now -[GSComboWindow deselectItemAtIndex:] still 
> > produces a
> > +       -[GSComboWindow tableViewSelectionDidChange:] call but the chain is
> > +	stopped in -[GSComboWindow validateSelection] by the 
> > _locationSelection
> > +       variable, then -[NSComboBoxCell selectItemAtIndex:] is not anymore
> > +       wrongly called.
> > +       
>
> Hi Quentin,

Hi Fred,

> the actual contents of your change look fine to me at first glance. 
> But the style of the ChangeLog needs improvement. In GNUstep we try to 
> comment on the place and type of the actual changes and less on the 
> reasons for the change and the still open issues (though they are 
> needed to be listed sometimes). So your change could have looked like 
> this:
>
>         * Source/NSComboBox.m (-mouseDown:): Reverted use of variable
>         clicked and renamed it to buttonClicked.
>         * Source/NSComboBox.m: (-runModalPopUpWithComboBoxCell:) removed
>         unneeded tableview setDelegate: call and remove observer for
>         notifications from the current window.
>         And so on...

Hmm… it is true that my ChangeLog entry isn't very well written. But I 
not really agree when you said that it is not a priority to comment on 
the reasons for the changes we do.

> Also there is no reason, why you had to add 
> trackMouse:inRect:ofView:untilMouseUp: to the NSComboBoxCell header. 
> This method is already declared in NSCell and we normally don't add 
> overwritten methods to the header. The only reason would be to place 
> the new documentation there, but this you put into the source file 
> (And again I agree that this is the better place to put it).

Totally agree with you here, but I have added the method to the header 
because the documentation process seems to ignore the methods 
documented in the source (.m file) and not declared in the header. 
Ideally we should improve the documentation tool in order to be able to 
remove the method declaration in the header (which is a temporary 
workaround). I know that I should have explained this stuff in the 
ChangeLog.

I will reorganize and detail this ChangeLog entry I think.

Otherwise we need to talk about NSTableView and especially the cell 
support (currently the implementation is really minimal and broken), 
there is also a bug in NSTableView you introduced in June which delays 
the selection highlighting until the mouse is up, such behavior is 
clearly not correct when the table view doesn't support drag. As a side 
effect, we get no highlighting in the combo box popup when we click on 
an entry.

Thanks for the feedback.

Quentin.

--
Quentin Mathé
address@hidden