Opened 15 years ago
Closed 15 years ago
#11491 closed defect (bug) (fixed)
Install.php confusing line
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 3.0 | Priority: | lowest |
Severity: | minor | Version: | 2.9 |
Component: | Upgrade/Install | Keywords: | has-patch |
Focuses: | 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)
#2
@
15 years ago
- 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.
#3
@
15 years ago
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..
#4
follow-up:
↓ 5
@
15 years ago
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.
#5
in reply to:
↑ 4
;
follow-up:
↓ 7
@
15 years ago
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.
#6
@
15 years ago
New patch, with updated phpdoc, better whitespace, etc.
No back compat issues. $step = 0, with two cases, case 0 or case 1.
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.