Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
On account of the backwards-incompatible changes made in PHP 7.0, it's helpful to move any calls to func_get_arg()
and func_get_args()
to the start of the function (even when the original location was safe in practice), in order to reduce cognitive load for the reader.
Making this change can also eliminate warnings (sometimes false-positive) from certain code linters. e.g. "[Warning] Function argument(s) returned by "func_get_args" might have been modified."
Refer to https://secure.php.net/manual/en/migration70.incompatible.php#migration7...
Comment | File | Size | Author |
---|---|---|---|
#15 | diff-views_ui-5c44b7f1^-15.txt | 721 bytes | jweowu |
#15 | views-func_get_args-3016530-15.patch | 986 bytes | jweowu |
| |||
#8 | views-func_get_args-3016530-8.patch | 3.42 KB | jweowu |
|
Comments
Comment #2
jweowu CreditAttribution: jweowu at Catalyst IT commentedComment #3
jweowu CreditAttribution: jweowu at Catalyst IT commentedMost calls to
func_get_args()
were already happening early. This patch addresses the remainder.Comment #4
DamienMcKennaThe change you made in views_revert_views() moves func_get_args() to later in the function?
Comment #5
jweowu CreditAttribution: jweowu at Catalyst IT commentedThe diff does make it look slightly odd, but that moves
$viewnames = _convert_csv_to_array(func_get_args());
to the top of the function.Comment #6
jweowu CreditAttribution: jweowu at Catalyst IT commentedIn the context of the reason behind this issue, it would also be entirely reasonable to change this to:
or, closer to the original code:
I'm happy to re-roll.
Comment #7
MustangGB CreditAttribution: MustangGB commentedFor future clarity might be best to be consistent and go with a
$args = func_get_args();
version.That said, I think DamienMcKenna just misread the patch file.
Comment #8
jweowu CreditAttribution: jweowu at Catalyst IT commentedComment #9
MustangGB CreditAttribution: MustangGB commentedSeems pretty straight forward.
Comment #10
DamienMcKennaCommitted. Thanks!
Comment #12
DamienMcKennaComment #13
jweowu CreditAttribution: jweowu at Catalyst IT commentedDamien, could you re-review the change I made to views_ui.module ?
Re-reading the patch, I'm aware that the way I've reordered things means that
drupal_build_form($form_id, $form_state);
is now called without the 'no_redirect' and 'rebuild_info' values which were previously in$form_state
when this happened.I obviously thought this was ok at the time, and now I'm not so sure.
drupal_build_form()
will triggerdrupal_rebuild_form()
ifisset($_SESSION['batch_form_state'])
, so the sequence change no longer looks safe to me.Comment #14
DamienMcKennaGood catch, yes, best to put those back.
Comment #15
jweowu CreditAttribution: jweowu at Catalyst IT commentedHere's the follow-up patch.
5c44b7f1 was the commit which added the preceding patch.
The new patch restores views_ui.module to its previous state (
5c44b7f1^
) except with (only) the code establishing$args
moved.diff-views_ui-5c44b7f1^-15.txt shows the difference between the previous state of views_ui.module and this new patch. If you commit this patch and check
git diff 5c44b7f1^ views_ui.module
you ought to see the same.Comment #17
DamienMcKennaAwesome, thank you!