Make WordPress Core

Opened 15 years ago

Closed 15 years ago

#11491 closed defect (bug) (fixed)

Install.php confusing line

Reported by: joshtime's profile joshtime 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)

11491.diff (9.2 KB) - added by nacin 15 years ago.
Inline docs and whitespace cleanup
11491.2.diff (9.0 KB) - added by nacin 15 years ago.
New patch is whitespace and code cleanup only -- no back compat issues.

Download all attachments as: .zip

Change History (10)

#1 @nacin
15 years ago

  • Component changed from Comments to Upgrade/Install

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.

@nacin
15 years ago

Inline docs and whitespace cleanup

#2 @nacin
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 @dd32
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: @hakre
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: @nacin
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.

@nacin
15 years ago

New patch is whitespace and code cleanup only -- no back compat issues.

#6 @nacin
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.

#7 in reply to: ↑ 5 @hakre
15 years ago

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.

#8 @nacin
15 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [13124]) install.php whitespace cleanup, inline docs, function @sinces, fixes #11491

Note: See TracTickets for help on using tickets.