Posted by pillarsdotnet on January 27, 2012 at 10:21pm
10 followers
Issue Summary
While submitting unrelated patches, I have noticed several core files that have trailing whitespace. This can easily be cleared up via the following one-line command:
perl -pi -e 's, *$,,' $(find core -regex '.*\.\(engine\|html\|inc\|info\|install\|js\|json\|module\|php\|script\|sh\|sql\|test\|txt\|xml\)')
Comments
#1
Great. Maybe it would also be possible to remove trailing whitespace from patch files uploaded to Drupal.org. But that's for another issue queue...
#2
Patch.
#3
(sigh) For real, this time.
#4
@#1 remove trailing whitespace from patch files
Super-terrific-wonderful-great idea for a 6.x/7.x contrib module.
#5
Hm. I'm all for this patch, but I'm wondering if we should be changing
core/modules/filter/tests/filter.url-input.txt and
core/modules/filter/tests/filter.url-output.txt
I guess all of the tests passed, but I also noticed there is no newline at the end of these files, so maybe the extra spaces were intentional in some way? I'm not sure, and I would say maybe we should avoid changing those files.
#6
Re-rolled as requested.
#7
Thanks! I think this is ready to commit.
#8
Yes, looks good. This has no chance of messing up anything, so safe fix!
#9
Regenerated after multiple core commits, but the resultant patch is identical to the one in #6.
(bump!)
#10
#6: remove-trailing-whitespace-1419298-6.patch queued for re-testing.
#11
Just a note that this affects much more than just .txt files and more than just documentation. It probably cannot be committed until the bug thresholds fall though.
#12
We're under thresholds so this is back on the table again. Assigning to Jennifer, and sending for a re-test.
#13
#6: remove-trailing-whitespace-1419298-6.patch queued for re-testing.
#14
I should mention though that my /vast/ preference is to have things like this handled through http://drupal.org/project/drupalcs so they can be caught *before* the patch is uploaded to d.o.
#15
The last submitted patch, remove-trailing-whitespace-1419298-6.patch, failed testing.
#16
Unassigning so someone can reroll this patch.
#17
Re-rolled.
#18
The last submitted patch, remove-trailing-whitespace-1419298-17.patch, failed testing.
#19
Unsubscribing. Since there are only twelve lines changed, and I can't imagine, after a thorough review of the patch, why it would break core, I find that I just don't care anymore.
#20
Just because you're frustrated is not necessarily a good reason to close the issue. Sigh.
#21
Rerolling (working on as part of DC2012 core sprints)
#22
#17: remove-trailing-whitespace-1419298-17.patch queued for re-testing.
#23
Generated by the command in opening post (against the current HEAD), excludes filter/tests/filter-input-* things (which intentionally include trailing whitespace).
#24
I applied the patch in #17 to an up-to-date checkout of 8.x and ran all tests, and it came back clean. Is there any chance there was another bug in 8.x that could have caused the tests to fail on this patch?
Re-queued for testing in hopes that it comes back OK this time. It sounds like pmitchell is re-creating the patch against the current HEAD, so I'll leave that to him/her.
#25
Not sure why pmitchell's worked and pillarsdotnet's did not, but this latest patch looks good to me and passes tests.
#26
I know this one is marked to remove trailing whitespace but what about tabs? In particular the misc folder has quite a few files that have tabs instead of spaces.
I didn't know if I should open a new issue or not so please let me know.
#27
The current patch is a monster. To avoid making it even bigger, I would recommend handling replacing tabs with spaces in a new issue so that this one can be put to bed.
#28
Setting this back to RTBC. The new issue is #1513540: Replace tabs with spaces from Drupal core files..
#29
#26: remove-trailing-whitespace-1419298-26.patch queued for re-testing.
#30
The last submitted patch, remove-trailing-whitespace-1419298-26.patch, failed testing.
#31
Reroll of #22