Opened 9 years ago
Closed 9 years ago
#34219 closed task (blessed) (fixed)
Move filters out of `rest_api_default_filters()`
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | normal | Version: | |
Component: | REST API | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
I just noticed the following docblock for rest_api_default_filters()
:
@internal This will live in default-filters.php
I don't know the details, but it looks like this was forgotten during the merge. The attached patch moves those filters to default-filters.php. In order to achieve this, rest_handle_deprecated_function
and rest_handle_deprecated_argument
are modified and also made private. It also introduces a new _rest_deprecated_trigger_error
function to disable errors during a REST request.
Note that I combined the two REST API sections in default-filters.php because (contrary to the inline comments) both are mostly for actions, not for filters. Having everything in one place makes it easier to find related hooks.
Another option is to just remove that @internal
comment and explain why it is that way.
Attachments (3)
Change History (14)
#3
in reply to:
↑ 2
;
follow-up:
↓ 4
@
9 years ago
Replying to rmccue:
These filters are only added on an API request, since we don't need them all the time.
Could we add if ( defined( 'REST_REQUEST' ) ) { ... }
to default-filters.php
?
#4
in reply to:
↑ 3
@
9 years ago
Replying to SergeyBiryukov:
Replying to rmccue:
These filters are only added on an API request, since we don't need them all the time.
Could we add
if ( defined( 'REST_REQUEST' ) ) { ... }
todefault-filters.php
?
REST_REQUEST
is defined in rest_api_init()
, which is hooked to init
and therefore executed after default-filters.php
.
#6
@
9 years ago
I'm -1 on the usage of the REST_REQUEST
constant to change behaviour. It makes it more difficult to test (as constants of course can't be toggled on / off), and would be more difficult for middleware / higher level plugins to use the REST API internally.
#7
@
9 years ago
Alright, let's just remove the @internal
note and update the documentation. See 34219.2.diff
#8
follow-up:
↓ 9
@
9 years ago
@swissspidy In 34219.2.diff you are going outside the scope of this ticket and are also removing the trailing spaces left in r35351. I don't know if that was intentional, or how to best strip the trailing spaces and retain an accurate history. Thoughts @johnbillion?
#9
in reply to:
↑ 8
@
9 years ago
Replying to rachelbaker:
@swissspidy In 34219.2.diff you are going outside the scope of this ticket and are also removing the trailing spaces left in r35351. I don't know if that was intentional, or how to best strip the trailing spaces and retain an accurate history. Thoughts @johnbillion?
Oops, not intentional. Was looking carefully not to include it and even wanted to create a new ticket for that.
#10
@
9 years ago
- Keywords commit added
In 34219.3.diff refreshed @swissspidy's last patch, limiting the scope.
These filters are only added on an API request, since we don't need them all the time. I think what we actually want here is to remove the
@internal
note, and add a note about why they're hooked separately into the function description.