![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 16 years ago
Closed 11 years ago
#14789 closed enhancement (maybelater)
Inconsistency in 'all' hook invocation
| Reported by: | hakre | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | Plugins | Version: | 3.0 |
| Severity: | normal | Keywords: | has-patch needs-refresh dev-feedback |
| Cc: | Focuses: |
Description
The 'all' hook (catchall hook for any hook name) is getting inconsistently called depending on how the concrete hook is being invoked: by apply_filters, apply_filters_ref_array, do_action or do_action_ref_array.
In the simple cases (apply_filters, do_action), the hook will get all parameters passed, in the ..._ref_array cases, the hook will get only two parameters passed in which the first parameter is the name of the hook (a.k.a. tag) and the second parameter is an array of all other parameters.
Every 'all' hook-function-callbacks should be called in the same way regardless whether which of the four invocation functions have been used on that pointcut.
Additionally all four routines share a lot of the same code which could benefit of a refactoring.
Attachments (7)
Change History (26)
#3
@
16 years ago
- Summary Inconsitency in 'all' hook invocation → Inconsistency in 'all' hook invocation
#5
@
16 years ago
Second Parameter Definition in Function Header of do_action() might be bogus: #14881
#6
@
16 years ago
First patch is the important one: http://core.trac.wordpress.org/attachment/ticket/14789/14789.patch
The other patches aren't the actual bugfix, they should go into a ticket on it's own.
#7
@
16 years ago
- Milestone Awaiting Review
- Resolution → maybelater
- Status new → closed
First patch might be ok.
Needs testing and validating with the unit test suite.
#8
@
15 years ago
- Keywords dev-feedback removed
- Resolution maybelater
- Status closed → reopened
14789.patch had been extensively tested, unit test suite run same with and w/o patch. If you have some specific test in mind, let me know.
Reference: #16661
#9
follow-up:
↓ 11
@
15 years ago
- Owner set to
- Status reopened → reviewing
The inconsistent spacing in 14789.patch makes it harder to read. No, really.
#11
in reply to: ↑ 9
@
15 years ago
Replying to scribu:
The inconsistent spacing in 14789.patch makes it harder to read. No, really.
That's right, the vertical rythm is broken. Will update it.
#14
in reply to: ↑ 13
;
follow-up:
↓ 15
@
13 years ago
Replying to c3mdigital:
Can you please justify your close suggestion?
#15
in reply to: ↑ 14
;
follow-up:
↓ 16
@
13 years ago
Replying to hakre:
Replying to c3mdigital:
Can you please justify your close suggestion?
To me this seems more like refactoring than a bug and hasn't had any traction in 3 years. I added 2nd opinion so someone else could give a suggestion on whether or not this ticket should stay open. I really don't see a bug here.
#16
in reply to: ↑ 15
@
13 years ago
- Keywords close removed
Replying to c3mdigital:
Replying to hakre:
Replying to c3mdigital:
Can you please justify your close suggestion?
To me this seems more like refactoring
No, this is not refactoring. Refactoring would mean that behavior is not changed even the code changed. Here the behavior is changed with this patch. Which by definition is not refactoring.
than a bug
The term bug is not really precise, as it's not refactoring can you please explain what you mean by that?
Basically filters and actions are hooks and the same.