This function provides an important feature to modules but the comparison is actually wrong and just returns the first module that implements hook_name.

This patch should fix it to provide the correct module name.

CommentFileSizeAuthor
#2 payment_get_modulename_comparison_1.patch1.01 KBneclimdul
#1 payment_get_modulename_comparison_0.patch636 bytesneclimdul
payment_get_modulename_comparison.patch834 bytesneclimdul

Comments

neclimdul’s picture

neclimdul
He/Him
commented
StatusFileSize
new payment_get_modulename_comparison_0.patch636 bytes

Stupid me... I diffed it against the CVS so if you've applied the payment_4.patch in the http://drupal.org/node/49933 then this might fail since i had that patch on the code.

It has no real relevance to the bug in question so for clarity here's a clean patch.

neclimdul’s picture

neclimdul
He/Him
commented
StatusFileSize
new payment_get_modulename_comparison_1.patch1.01 KB

And since I apparently didn't get enough sleep last night, here's the unified diff version. *sigh* sorry

sime’s picture

sime
any/any
Location Melbourne
commented

I notice in your patch you mention:
//get the list of modules. It'd be really cool if I could confine it only to payment modules.

Just wondering what you see as the limitation of this code. Performance?

neclimdul’s picture

neclimdul
He/Him
commented

Yeah that's actually from the cvs. I tend to agree with sime though. I think we're taking the wrong approach in introducing this. We should probably keep track of the module name in the transaction and call the api to get the display name when its needed rather than the other way around. This keeps us from going through the module list to get the name. However, if we like the idea of this function I can make a patch to cache hits per page request since right now its getting called several times on some pages.

For reference, here's the issue that brought this behavior into payment http://drupal.org/node/51321.

sime’s picture

sime
any/any
Location Melbourne
commented

Sorry, you're right, it wasn't your comment. My question about performance assumed it was your comment. That said, calling module_list() a few times is not too bad - as module_list() caches the list in a static variable.

Having had ongoing problems with ccard, even with this patch, I have almost come to grips with this issue. I'll be adding more comments about this to http://drupal.org/node/49933.

sime’s picture

sime
any/any
Location Melbourne
commented

See: http://drupal.org/node/51852

sime’s picture

sime
any/any
Location Melbourne
commented
Status: Needs review » Closed (duplicate)

Sorry, forgot to mark duplicate