[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Gnustep-cvs] Commit Update

From: Quentin Mathé
Subject: Re: [Gnustep-cvs] Commit Update
Date: Sat, 14 Aug 2004 17:08:44 +0200

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 Mathé

reply via email to

[Prev in Thread] Current Thread [Next in Thread]