Jean Jordaan wrote:
Hi Martin& all
Current:
<monkey:patch
description="Add XML-RPC capability to SomeModule"
module="Products.SomeProduct.SomeModule"
original="readRemoteProject"
ignoreOriginal="True"
replacement=".SomeModule.readRemoteProject"
/>
Suggested:
<monkey:patch
description="Add XML-RPC capability to SomeModule"
module="Products.SomeProduct.SomeModule"
checkOriginal="False"
patch=".SomeModule.readRemoteProject"
/>
+1
I'm not sure what ignoreOriginal is doing in this case; I'd probably just
ignore it.
In the original, 'ignoreOriginal="True"' is required because the
original doesn't exist. I'm adding a new function. If you ignore it,
you'll get an exception.
Yep. What I'm saying is that if you didn't specify an original, then
ignoreOriginal shouldn't be needed either. So the way to add a new
function to a module is:
<monkey:patch
description="..."
module="foo.bar"
patch=".new.function"
/>
In my suggested version, you have to switch off the existence check that
'patch' normally does, because in this case you know that there is no
original. That said, I'd like to suggest 'new' instead of
'checkOriginal'::
<monkey:patch
description="Add XML-RPC capability to SomeModule"
module="Products.SomeProduct.SomeModule"
new="True"
patch=".SomeModule.readRemoteProject"
/>
Yeah, that's even more explicit, which is probably good. Or how about just:
<monkey:patch
description="Add XML-RPC capability to SomeModule"
module="Products.SomeProduct.SomeModule"
new=".SomeModule.readRemoteProject"
/>
I think I like that better. It's explicit and less typing.
If I simply want to replace an existing function of the same name,
checking for the original::
<monkey:patch
description="Add XML-RPC capability to SomeModule"
module="Products.SomeProduct.SomeModule"
patch=".SomeModule.readRemoteProject"
/>
-1
I think you should have to explicitly say what you're patching. Just
looking for something with the same name is too magical. The whole
point of collective.monkeypatcher is to make everything very explicit.
It is explicit; there's no magic. The documentation of 'patch' will say
that it expects to find a member of the same name. If that member
doesn't exist, monkeypatcher will raise an exception. That will
encourage keeping the names of the patch and the original the same,
which will make the patching product more readable and amenable to e.g.
ctags, because the names from the original API are preserved.
Sorry, I still don't like it. I think we can encourage people to use the
same name, but I think that if you want to replace something, you should
explicitly say both the name of the thing you're replacing and the name
of the thing you're replacing it with.
If I'm replacing an existing function, and for some reason the
'description' attribute is insufficient to describe what I'm doing and I
want to name my method something different::
<monkey:patch
description="Add XML-RPC capability to SomeModule"
module="Products.SomeProduct.SomeModule"
original="readRemoteProject"
patch=".SomeModule.readRemoteProject_myownspecialweirdname"
/>
This is the current basic syntax, no? I think this is the appropriate way
still for both this and the case above where the patch function and the
original function happened to have the same name.
Yes, this is like the current. I think the repeat is makework and a
chance for typos though.
It's validated, so that's not really an issue. And it's not a repeat.
It's only a repeat if you happen to subscribe to the convention that the
two methods must have the same name, which they don't. The "patch" is
referring to an entirely different method with a different module path.
Only the last bit (the function name) is the same, except it doesn't
need to be the same.
I hope you agree that documentation like "by
default, 'patch' replaces a member of the same name on the patched
object" makes things clear enough.
I don't. :) Well, I do, but it assumes that people reading the
<monkey:patch /> statement has read and recall the documentation. I'd
rather the statement was explicit.
Of course, I may be completely missing something that invalidates my
proposal .. please tell me if I am!
I think it's mostly just a matter of balance. If you are monkey
patching, I think you deserve to spell out very, very carefully exactly
what you're replacing. If you're doing it so much that re-typing this is
causing you pain, you have no sympathy from me.
Martin
--
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book
_______________________________________________
Product-Developers mailing list
[email protected]
http://lists.plone.org/mailman/listinfo/product-developers
