github / Rebel

Cocoa framework for improving AppKit
Other
1.13k stars 111 forks source link

Concerns about -prepareForReuse in RBLTableCellView #102

Closed jwilling closed 11 years ago

jwilling commented 11 years ago

I'll tread lightly here due to NDA, but I think this is important to get figured out before [Sea Lion] is released.

In RBLTableCellView I added a -prepareForReuse method, which is called when the superview changes. This is all fine and dandy, except for the fact that apparently this method has already existed since 10.7 on NSView, and has just been made public.

I have done no actual testing of this, however from just introspection it seems like this might be a potential concern.

jspahrsummers commented 11 years ago

Let's rename it, intentionally as a breaking change.

jwilling commented 11 years ago

Any suggestions on a new method name? Just to throw a couple out there:

I believe we already went over some names in the previous thread where this was originally created as well.

alanjrogers commented 11 years ago

-rbl_prepareForReuse

dannygreg commented 11 years ago

Yeah let's just have a prefix.

jwilling commented 11 years ago

I'll be happy to make a pull request for this.

Also, here's another thought. What about just removing this class and just adding a category that declares the method so people can just use the built-in method? Alternatively, we could still have a prefixed method that is called internally from the system's implementation.

jspahrsummers commented 11 years ago

I can't actually find any confirmation that -prepareForReuse is a thing now. Wasn't able to find it in a header or in the docs for the 10.9 SDK.

In any case:

What about just removing this class and just adding a category that declares the method so people can just use the built-in method?

Only if it's explicitly documented to have existed since 10.7.

jwilling commented 11 years ago

In NSView.h:

/* The View Based NSTableView allows views to be reused. Sometimes it is necessary to 
prepare a view with some initial state before it is to be reused. This method can be overridden 
to allow a view to be prepared back to the default state. Override this method to do the 
preparation. By default, NSView will do some setup, such as setting the view to not be
hidden and have a 1.0 alpha. It is important to call super to get this work done. This method 
was made public in 10.9, but exists back to 10.7.
 */
- (void)prepareForReuse NS_AVAILABLE_MAC(10_7);

(feel free to delete this if you think it's violating NDA)

jspahrsummers commented 11 years ago

lol, NSView.

jwilling commented 11 years ago

Yeah pretty classic.

indragiek commented 11 years ago

:+1:

jwilling commented 11 years ago

So, what's the consensus? Here are our options:

jspahrsummers commented 11 years ago

Prefix the current method, to avoid breakage when super isn't called like the official method expects. However, we should document that they can use -prepareForReuse instead.

jwilling commented 11 years ago

:+1: