effigy
effigy copied to clipboard
jferris
Remove the selection proxy object
I've replaced the selection proxy object, and replaced it with a simple clone. The proxy object wasn't really pulling its weight, and it was locking us into having a selector at the beginning of every method signature, which made things like #first impossible
This PR also removes the optional selector (e.g. .text('h1', 'foo'))
as an argument to all of the various methods, in favor of
.find('h1').text('foo')
Does it make sense to move a bunch of the manipulation stuff into Selection instead of removing it? It seems like this is moving in the direction of having a mono-class library. Now that you can't call text with a selector, it doesn't really make sense to call text before find, and moving them out of View would prevent that.
Yeah, I think everything related to current_context wants to live in another class. It'd be cool to do it in a way that exposes the enumeration more, as well. Do you think it would make sense to do that as a separate PR? Since it probably won't end up being a proxy object, so the existing code from selection will still go away regardless.
I think I'd want to see that the approach works before merging. I agree that the proxy approach isn't great, and that most of the internals of Selection should go away, but I worry that merging this as-is doesn't improve the API; it just replaces the old unintuitive API with a new unintuitive API. Does that make sense?
Yeah, I agree with you. I'll take a swing at it tomorrow.
On Wed, May 8, 2013 at 3:30 PM, Joe Ferris [email protected] wrote:
I think I'd want to see that the approach works before merging. I agree that the proxy approach isn't great, and that most of the internals of Selection should go away, but I worry that merging this as-is doesn't improve the API; it just replaces the old unintuitive API with a new unintuitive API. Does that make sense?
— Reply to this email directly or view it on GitHubhttps://github.com/jferris/effigy/pull/18#issuecomment-17635212 .
Thanks, Sean Griffin
@jferris Do you think that the block API with find still makes sense given these changes?
No, not really, especially if you can perform multiple manipulations on a selector chain (ie find('h1).add_class('title').text(page.title)).