Coding standards for file ending newlines

you will get a "no ending newline" from patch if that's missing imo... it does no harm and sometimes you need to fight your editor to not add one.

The only file in a clean installation of HEAD matching the pattern '\W+\Z' is misc/jquery.js, but it has a trailing end line. Can we consider this fixed?

While I'm at it, let's set the status to move this issue along.

Does someone want to confirm that all core files have a trailing newline?

misc/jquery.js is the only one that comes up for me, too.

Went through the first 15 or so changes and everything checked out. Patch applies cleanly and works fine. Follows standards. Don't really know how to review it except to say that :).

Let's retest.

Bump..

The patch no longer works due to all the intervening changes to the code. Is there some automated process to pick out all files in core that don't end in one new line and make them end in exactly one new line?

I'd reroll this if I knew how, but I suspect I'm going to waste a lot of time playing around with line endings...

rerolled, temporarily critical, due to amount of changes.

this patch was made possible thanks to the song Code Monkey by Jonathan Coulton
http://www.jonathancoulton.com/2006/04/14/thing-a-week-29-code-monkey/

what happened bot?
- Failed to install HEAD
- WTF??? stupid bot!!!

do not trust the bot: http://testing.drupal.org/node/33

Setting to previous status - testbot was broken (failed to install).

and it passes, testbot is not broken anymore :D

note that there are no new-line warnings in the adds, and all newline removals were duplicate newlines at the end of the file.

Thanks for that.

How'd you find the files with newline problem? Was it just figuring out the right regex string?

did not even used regex ;)

0) remove CVS and binary (images) files!!!
1) append unique identifier to all and every file
2) search and replace magic
3) create intermediate.patch
4) check out CVS again
5) apply intermediate patch to clean CVS checkout
6) no problems :D
7) cvs diff
8) patch ready!

Oh. I see..

I was hoping for a quick way of doing it. =P Turns out, grep can't search across multiple lines anyway...

This isn't critical.

I'm also heavily considering "won't fix"ing this. I don't think we'll ever reach a point where everyone's editors aren't (re-)introducing/stripping these trailing whitespaces, so this kind of feels like a fight we're never going to win.

At least, we should add this to the coding standards.

/me hates missing newline at end of file

sun: That sounds reasonable. Want to take care of it?

Dave: I know, but there are obviously editors out there that are stripping out trailing newlines, or this wouldn't come up all the time. And rather than core committers doing this, the testing bot should do this. Then it could be fixed way before it ever hits RTBC. I know there must be an issue for this somewhere but I can't find it.

Moving to Coder's queue.

This issue was marked 'novice' on topic that doesn't have a clear decision made and might be already irrelevant. Shall we mark this closed(won't fix)? Or is there an easy means for a novice to actually fix this :)

Let's not mark this closed (won't fix). However, with the current review code, it may not be a novice item.

Removing 'novice' tag and setting to "close won't fix" as per #37, #29, and the fact that this might be solvable.... or by some automated solution listed/discussed in #34. But I'm happy to have this reopened if someone feels strongly enough that it should still be done.