Minor whitespace fix. Also needs committed to Drupal 7. Applies cleanly to both.

CommentFileSizeAuthor
#11 trailing-whitespace.patch264 bytestr
#4 1263882-2-authorize-php.patch1 KBcweagans
#2 1263882-authorize-php.patch1.15 KBtr
authorize_php_code_style.patch366 bytescweagans

Comments

marcingy’s picture

marcingy commented
Status: Needs review » Reviewed & tested by the community

Looks fine to me.

tr’s picture

tr commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 1263882-authorize-php.patch1.15 KB

There's also blank space at the end of the script, and a few doxygen comments that needed tweaking for documentation standards. Modified patch fixes these. Applies cleanly to D8 and D7.

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented

Per http://drupal.org/coding-standards#indenting, there is supposed to be an empty line at the end of files

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented
StatusFileSize
new 1263882-2-authorize-php.patch1 KB

New patch.

tr’s picture

tr commented

No, that says last line should end with a newline, which my patch does. Before the patch, authorize.php ends with *two* newlines, which potentially causes the dreaded "Header already sent" error. There should never be a blank line at the end of any piece of Drupal code.

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented
Status: Needs review » Reviewed & tested by the community

Ah, you're right - I misread the patch. webchick, Dries: #2 is what should be committed

Also, FWIW, the "Header Already Sent" error wouldn't be caused by an extra empty line inside the scope of php. If you were to add a ?> and a new line after that, then you'd have the Header Already Sent error.

I'm going to set this back to RTBC. Again, this is for the patch on #2.

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
commented
Title: Fix authorize.php code style » Code style: authorize.php
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Issue tags: +Needs backport to D7

Fixing tag.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 7.x and 8.x. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

tr’s picture

tr commented
Category: task » bug
Priority: Minor » Normal
Status: Closed (fixed) » Reviewed & tested by the community
StatusFileSize
new trailing-whitespace.patch264 bytes

The wrong patch was committed. See #6 where it says the correct patch is #2.

There is still a blank line at the end of authorize.php. Attached patch corrects this. Works for both D8 and D7.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 7.x and 8.x. Thanks.

Automatically closed -- issue fixed for 2 weeks with no activity.