Download & Extend

Code style: authorize.php

Project:Drupal core
Version:8.x-dev
Component:base system
Category:bug report
Priority:normal
Assigned:cweagans
Status:closed (fixed)
Issue tags:needs backport to D7

Issue Summary

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

AttachmentSizeStatusTest resultOperations
authorize_php_code_style.patch366 bytesIdlePASSED: [[SimpleTest]]: [MySQL] 32,862 pass(es).View details

Comments

#1

Status:needs review» reviewed & tested by the community

Looks fine to me.

#2

Status:reviewed & tested by the community» needs review

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.

AttachmentSizeStatusTest resultOperations
1263882-authorize-php.patch1.15 KBIdlePASSED: [[SimpleTest]]: [MySQL] 32,865 pass(es).View details

#3

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

#4

New patch.

AttachmentSizeStatusTest resultOperations
1263882-2-authorize-php.patch1 KBIdlePASSED: [[SimpleTest]]: [MySQL] 32,855 pass(es).View details

#5

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.

#6

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.

#7

Title:Fix authorize.php code style» Code style: authorize.php

#8

Issue tags:+needs backport to D7

Fixing tag.

#9

Status:reviewed & tested by the community» fixed

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

#10

Status:fixed» closed (fixed)

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

#11

Category:task» bug report
Priority:minor» normal
Status:closed (fixed)» reviewed & tested by the community

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.

AttachmentSizeStatusTest resultOperations
trailing-whitespace.patch264 bytesIdlePASSED: [[SimpleTest]]: [MySQL] 29,936 pass(es).View details

#12

Status:reviewed & tested by the community» fixed

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

#13

Status:fixed» closed (fixed)

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