Re: [Gnustep-cvs] r24007 - in /libs/gui/trunk: ChangeLog Source/GSDragView.m Source/NSCell.m Source/NSTableView.m

2006-11-06 Thread Fred Kiefer
matt rice schrieb:
> Author: ratmice
> Date: Wed Nov  1 10:50:00 2006
> New Revision: 24007
> 
> URL: http://svn.gna.org/viewcvs/gnustep?rev=24007&view=rev
> Log:
> * Source/NSTableView.m: Add new private methods.
> (-mouseDown:): Reorganize and don't track cells until dragging
> has been ruled out.
> * Source/NSCell.m (trackMouse:inRect:ofView:untilMouseUp:):
> Handle events no longer in the queue.
> * Source/GSDragView.m: Change NSLog to NSDebugLLog.
> 

Hi Matt,

first of all, thank you for doing all this great work in cleaning up the
behaviour of NSTableView. This is a very complex class and it is so
unbelievable hard to get things right there.
A few comments to this specific change.

- It looks like you are not using the standard GNUstep formatting rules.
I must admit that I am not following them all the time, but the way you
place your blanks or rather leave them out, should be adopted to the to
be more readable. One space character after a colon should be used :-)

- The new method _startDragOperationWithEvent: should pass on the offset
for the dragging and not mix this with the start position.

- In the new tracking method, we only check if the column is editable,
shouldn't we use the method that checks for the cell to be editable?
(BTW in this the comments are still wrong)

- A bit more comments in the mouseDown: method would be helpful. This is
so complex and there are so many different things we tried before, that
we really need to explain what is happening and why. I still don't
understand if the dragging is stealing events from the tracking or the
other way around or even both may happen.

You see, this are less things that are wrong, but rather stuff that
needs to be clearer to guarantee that the progress you are making here
will last.

Cheers
Fred


___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
http://lists.gnu.org/mailman/listinfo/gnustep-dev


Re: [Gnustep-cvs] r24007 - in /libs/gui/trunk: ChangeLog Source/GSDragView.m Source/NSCell.m Source/NSTableView.m

2006-11-06 Thread Matt Rice


--- Fred Kiefer <[EMAIL PROTECTED]> wrote:

> matt rice schrieb:
> > Author: ratmice
> > Date: Wed Nov  1 10:50:00 2006
> > New Revision: 24007
> > 
> > URL:
>
http://svn.gna.org/viewcvs/gnustep?rev=24007&view=rev
> > Log:
> > * Source/NSTableView.m: Add new private
> methods.
> > (-mouseDown:): Reorganize and don't track
> cells until dragging
> > has been ruled out.
> > * Source/NSCell.m
> (trackMouse:inRect:ofView:untilMouseUp:):
> > Handle events no longer in the queue.
> > * Source/GSDragView.m: Change NSLog to
> NSDebugLLog.
> > 
> 
> Hi Matt,
> 
> first of all, thank you for doing all this great
> work in cleaning up the
> behaviour of NSTableView. This is a very complex
> class and it is so
> unbelievable hard to get things right there.
> A few comments to this specific change.
> 
> - It looks like you are not using the standard
> GNUstep formatting rules.
> I must admit that I am not following them all the
> time, but the way you
> place your blanks or rather leave them out, should
> be adopted to the to
> be more readable. One space character after a colon
> should be used :-)

ahh, yeah, i really can't stand that way of doing
method names, as my brain always interprets
[scrooge bah: humbug] as multiple methods with no
arguments I find it much easier/quicker to distinguish
the argument from the method name, anyhow... ok, its
the law i guess :(

> 
> - The new method _startDragOperationWithEvent:
> should pass on the offset
> for the dragging and not mix this with the start
> position.

ok, i noticed this too but wasn't sure 

> 
> - In the new tracking method, we only check if the
> column is editable,
> shouldn't we use the method that checks for the cell
> to be editable?
> (BTW in this the comments are still wrong)

ahh hm, i'm not sure, I didn't think so because we are
calling trackMouse... this might not cause any editing
it could be 

for instance
in the -willDisplayCell:... you could set the cells
target/action, based on some criteria and the cell is
never edited actually.

based on some criteria and tracking the mouse sends
the action, no editing involved,

then it checks if theres any change in the cells
object value.

i consider -isEditable: only useful for
-editWithFrame: and possibly a cell classes internal
usage in trackMouse:

> 
> - A bit more comments in the mouseDown: method would
> be helpful. This is
> so complex and there are so many different things we
> tried before, that
> we really need to explain what is happening and why.
> I still don't
> understand if the dragging is stealing events from
> the tracking or the
> other way around or even both may happen.

only if dragging is enabled, the dragging code 
is stealing events, and then sending events which
happened in the past to NSCell.

> 
> You see, this are less things that are wrong, but
> rather stuff that
> needs to be clearer to guarantee that the progress
> you are making here
> will last.

indeed

> 
> Cheers
> Fred
> 




 

Do you Yahoo!?
Everyone is raving about the all-new Yahoo! Mail.
http://new.mail.yahoo.com


___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
http://lists.gnu.org/mailman/listinfo/gnustep-dev


Re: [Gnustep-cvs] r24007 - in /libs/gui/trunk: ChangeLog Source/GSDragView.m Source/NSCell.m Source/NSTableView.m

2006-11-06 Thread Matt Rice


--- Matt Rice <[EMAIL PROTECTED]> wrote:

> 
> 
> --- Fred Kiefer <[EMAIL PROTECTED]> wrote:
> 
> > matt rice schrieb:


> 
> > 
> > - In the new tracking method, we only check if the
> > column is editable,
> > shouldn't we use the method that checks for the
> cell
> > to be editable?
> > (BTW in this the comments are still wrong)
> 
> ahh hm, i'm not sure, I didn't think so because we
> are
> calling trackMouse... this might not cause any
> editing
> it could be 
> 
> for instance
> in the -willDisplayCell:... you could set the cells
> target/action, based on some criteria and the cell
> is
> never edited actually.
> 
> based on some criteria and tracking the mouse sends
> the action, no editing involved,
> 
> then it checks if theres any change in the cells
> object value.
> 
> i consider -isEditable: only useful for
> -editWithFrame: and possibly a cell classes internal
> usage in trackMouse:
> 

this example was kindof contrived,
a better example might be a selectable non editable
text field cell, where you can copy the text fields
contents.

trackMouse: should allow you to select the text.





 

Cheap talk?
Check out Yahoo! Messenger's low PC-to-Phone call rates.
http://voice.yahoo.com


___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
http://lists.gnu.org/mailman/listinfo/gnustep-dev


Re: [Gnustep-cvs] r24007 - in /libs/gui/trunk: ChangeLog Source/GSDragView.m Source/NSCell.m Source/NSTableView.m

2006-11-06 Thread Matt Rice


--- Fred Kiefer <[EMAIL PROTECTED]> wrote:

> matt rice schrieb:

> - A bit more comments in the mouseDown: method would
> be helpful. This is
> so complex and there are so many different things we
> tried before, that
> we really need to explain what is happening and why.
> I still don't
> understand if the dragging is stealing events from
> the tracking or the
> other way around or even both may happen.

sorry i'm very tired and was wrong in my previous
email
both will happen when dragging is possible.

a) when it will wait until dragging has been ruled
out.
to track cells or select rows
then it sends cells the old event

b) after tracking the mouse on the cell, it works with
an event that happened during the cells tracking to
determine if the mouse has gone up.

b) does not seem neccessary but we can't return early
currently because row selection happens at the end of
the loop and the loop would never return if we didn't.

this also causes row selection not happen until mouse
up or mouse drag, and should be fixed.
 
Cheers



 

Sponsored Link

Mortgage rates near historic lows: 
$150,000 loan as low as $579/mo. Intro-*Terms 
https://www2.nextag.com/


___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
http://lists.gnu.org/mailman/listinfo/gnustep-dev


Re: [Gnustep-cvs] r24007 - in /libs/gui/trunk: ChangeLog Source/GSDragView.m Source/NSCell.m Source/NSTableView.m

2006-11-07 Thread Matt Rice


--- Fred Kiefer <[EMAIL PROTECTED]> wrote:

> matt rice schrieb:
> > Author: ratmice
> > Date: Wed Nov  1 10:50:00 2006
> > New Revision: 24007
> > 
> > URL:
>
http://svn.gna.org/viewcvs/gnustep?rev=24007&view=rev
> > Log:
> > * Source/NSTableView.m: Add new private
> methods.
> > (-mouseDown:): Reorganize and don't track
> cells until dragging
> > has been ruled out.
> > * Source/NSCell.m
> (trackMouse:inRect:ofView:untilMouseUp:):
> > Handle events no longer in the queue.
> > * Source/GSDragView.m: Change NSLog to
> NSDebugLLog.
> > 
> 
> - It looks like you are not using the standard
> GNUstep formatting rules.
> I must admit that I am not following them all the
> time, but the way you
> place your blanks or rather leave them out, should
> be adopted to the to
> be more readable. One space character after a colon
> should be used :-)


ok i commited some changes to this, i'll update the
rest of my patches accordingly

> 
> - The new method _startDragOperationWithEvent:
> should pass on the offset
> for the dragging and not mix this with the start
> position.
> 

i'm really not sure what exactly you mean by this,
that code is unchanged only moved into a private 
method

since it was lengthy and self contained
to reduce nesting and the size of mouseDown:
and its only ever called once, not in a 'tight loop'.



> - In the new tracking method, we only check if the
> column is editable,
> shouldn't we use the method that checks for the cell
> to be editable?
> (BTW in this the comments are still wrong)

ok, i think i see what you mean, for some reason i
thought you meant we should check the cell for
editablity before *tracking* on the cell

this seems to be very much a corner case,
i'm actually not sure what should happen
it means, 

the table column is editable,
the cell is not,
the cells object value has changed during tracking,

seems quite rare,
and i'm not exactly sure what kind of behaviour one
would expect, ignore the object value change, or
expect it to be changed,

we didn't actually edit the cell, we told it to track
the mouse, and it decided to change the object value
it seems to me like the cells decision
and if it doesnt want to change the object value if it
isn't editable, it should handle that in the mouse
tracking

> - A bit more comments in the mouseDown: method would
> be helpful. This is
> so complex and there are so many different things we
> tried before, that
> we really need to explain what is happening and why.
> I still don't
> understand if the dragging is stealing events from
> the tracking or the
> other way around or even both may happen.

ok i added some of these





 

Sponsored Link

Get a free Motorola Razr! Today Only! 
Choose Cingular, Sprint, Verizon, Alltel, or T-Mobile. 
http://www.letstalk.com/inlink.htm?to=592913


___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
http://lists.gnu.org/mailman/listinfo/gnustep-dev