A lot of .inc files in /core/includes (D8) and /include (D7) have trailing whitespace at the end of the file. These patches fix this.

CommentFileSizeAuthor
#11 trailing-whitespace-7.patch2.46 KBjhodgdon
trailing-whitespace-D7.patch2.46 KBTR
trailing-whitespace.patch3.02 KBTR
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TR’s picture

TR CreditAttribution: TR commented
Status: Active » Needs review
aspilicious’s picture

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

Ok this is a no brainer

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Closed (works as designed)

Not quite:

All text files should end in a single newline (\n). This avoids the verbose "\ No newline at end of file" patch warning and makes patches easier to read since it's clearer what is being changed when lines are added to the end of a file.

http://drupal.org/coding-standards

TR’s picture

TR CreditAttribution: TR commented
Status: Closed (works as designed) » Reviewed & tested by the community

"single newline" means *one* newline. This patch fixes those files which have *two* newlines.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

I think this patch is correct. (If it weren't I believe it would be full of added \No newline at end of file barf.)

droplet’s picture

droplet CreditAttribution: droplet commented

Cool. I think we need a plan to postponed all these issues (spaces / tabs / comment head format) and fix them at very the end of D8 development.

(but this one also exist in D7)

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

I think we need a plan to postponed all these issues (spaces / tabs / comment head format) and fix them at very the end of D8 development.

Actually, now is the best time to fix this sort of stuff, when there's no scary D8 deadlines looming. See also #1310084: [meta] API documentation cleanup sprint and the core gates. Ensuring that code conforms to our style and documentation standards should not be postponed to the last minute; it should be a consistent part of the development process.

droplet’s picture

droplet CreditAttribution: droplet commented

Yeah, I agreed to clean up documentation / big standards issue right now but for above one is a small case.

anyway, another coding standards issue, affected around 124 files: #1109202: [meta] Fix coding standards violations across Drupal core

TR’s picture

TR CreditAttribution: TR commented

The trailing whitespace fixed by these patches is not addressed in #1109202: [meta] Fix coding standards violations across Drupal core, and these patches do not conflict with #1109202: [meta] Fix coding standards violations across Drupal core. This issue deals with a trivial problem I noticed while rolling a patch for a different issue, and it's something that shouldn't need discussion.

IMO it's far easier to fix the little things as we go along - patches like this have little to no impact on other works in progress.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs backport to D7

Committed/pushed to 8.x. There's no tag here, but there's a 7.x patch so adding the tag and setting CNR against that branch.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Reviewed & tested by the community
FileSize
trailing-whitespace-7.patch2.46 KB

Re-uploading d7 patch with a new name so the bot can test it, and as it looks fine, setting to RTBC (pending test bot passage).

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Priority: Normal » Minor
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x, though I'd really love to stop committing patches like this and instead get automated checks built into testbot via http://drupal.org/project/coder and/or http://drupal.org/project/drupalcs. As of http://jthorson.doesdrupal.com/node/34 testbot can now do "Advisory" code reviews for just this purpose.

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

acb’s picture

acb CreditAttribution: acb commented

TR-- Been trying to get in touch. Could you please check your drupal messages/email/spam folder? Thank you! -ACB