Opened 3 years ago
Closed 3 years ago
#11491 closed defect (bug) (fixed)
Install.php confusing line
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Priority: | lowest | Milestone: | 3.0 |
| Component: | Upgrade/Install | Version: | 2.9 |
| Severity: | minor | Keywords: | has-patch |
| Cc: |
Description
In wp-admin/install.php, at line 85-87 of the 2.9 is somewhat confusing.
switch($step) {
case 0:
case 1: // in case people are directly linking to this
// display_header();
Suggestions:
Add a note saying Fall Through
Make the comment more clear
Attachments (2)
Change History (10)
- Keywords has-patch added
- Milestone changed from Unassigned to 3.0
Patch adds whitespace and updates some basic inline docs.
Also gets rid of $step = 0. Only if someone was actually linking to install.php?step=0 would this break backwards compatibility, in which case "case 0:" should be added back to the switch.
IMO: Leave the step orders alone, Some 3rd party installers set it and expect it that way.. Theres no harm in using Step <default of 0 or 1> and 2..
Patch adds too much spaces. Functions do not need space on both sides of the bracket, that's only need for some language constructs like if. Other parts of the patch are looking ok to me. It's nice to see someone took the time to figure out the @since version numbers IMHO.
Replying to hakre:
Patch adds too much spaces. Functions do not need space on both sides of the bracket, that's only need for some language constructs like if.
That's the WP coding standard.
I think it looks cleaner to avoid padding spaces inside __() and possibly _e() function calls. Indeed, the core adheres to this more than it doesn't -- 4,324 instances of ('blah') or _e('blah'), to 577 __( 'blah' ) and _e( 'blah' ). Otherwise, I'm all for the extra whitespace and so are the core devs.
New patch, with updated phpdoc, better whitespace, etc.
No back compat issues. $step = 0, with two cases, case 0 or case 1.
Replying to nacin:
That's the WP coding standard.
Yes, you're right, I made that comment in error. I currently adopt my patches because of your feedback.

There's nothing wrong with these lines. When you read the entire install.php file (only 156 lines), it makes sense.
Lines 23-26: if $_GETstep? is set, $step equals $_GETstep?. Otherwise, $step equals 0.
There's two steps to the admin process, admin.php and admin.php?step=2, which cause $step to be 0 and 2, respectively. The check for $step = 1 is "in case people are directly linking to" admin.php?step=1.