By earlofsandwich on

I'm slightly confused about the module coding standard which states "All text files should end in a single newline (\n)"
http://drupal.org/node/318#indenting

Does this mean

}\n

or does it mean

}\n
\n

(hope that makes sense)

My interpretation is that the top example is the correct one as the file does finish with a single newline character, NOT the second one which actually finishes with two newlines. I ask because PAReview.sh is bumping my code complaining that I don't end in a single newline.

Many thanks

⋅ Categories: Drupal 6.x, Drupal 7.x

Comments

Drave Robber’s picture

Drave Robber commented

Your interpretation is correct, although the real meaning of that statement is "\n (Unix style), not \r\n (Windows style)". If you're using Win, look into editor settings (can't tell where exactly, haven't used Win myself for years).

earlofsandwich’s picture

earlofsandwich commented

Thanks Drave,

I think I was getting confused because it says a 'single newline' (which could imply a line on its own) rather than a 'single newline character' - which would be at the end of a line of code.

I think I should maybe post this in the documentation pages instead?

neokrish’s picture

neokrish commented

the documentation means 

}\n
\n

For eg. take up index.php file that comes with drupal 7, it has 21 lines of code but it actually has 22 lines (the extra one blank line. The same goes with update.php. The best way to identify and understand the coding standards is to open up the drupal core and compare it :-)

earlofsandwich’s picture

earlofsandwich commented

Thanks for the suggestion about checking core, should have thought of that one!

If you examine the D7 files with line endings turned on they are just }\n - Komodo IDE/EDIT has a great show EOL marker view.

The index.php has 21 lines of text with the \n at the very end of line 21 giving one extra (completely empty line) number 22 - I think we are both saying the same thing that basically there being one line of space at the bottom.

If it was }\n\n then when you moved the cursor to the bottom of the file it would sit 2 lines below which would not match core and would be 23 lines long!

Pedantic, I know, but this is why I was ending up in a tizz. Thanks all.

earlofsandwich’s picture

earlofsandwich commented

Hello again, I think I got the wrong end of the stick last post - It does indeed appear to be }\n\n - Thanks doitdave

This does appear to differ from what is in Core though, which, at least on my PC, only has a single newline at the end of its files... (Its not out of the question that my Windows GIT setup is stripping the final \n?)

Anyway, the long and the short of it is that a module/readme should end in a completely blank line - as in have one completely blank line after all the rest of the code, or two newline characters.

This seems to be what PAReview.sh validates to.

Phew. Thank you everyone.

tr’s picture

tr commented

Nonsense. The documentation means

}\n

It has always meant that, and that is the way all Drupal core is written. PAReview.sh has a flawed test that was introduced just 12 days ago that tells you there should be an extra blank line at the end, which is what started all this confusion, but that's just a PAReview.sh bug - nothing has changed in the coding standards.

Specifically, to use your example, index.php has 21 lines. The last line is:

menu_execute_active_handler();\n

I really don't know where you come up with "22 lines". Perhaps you're using Windows and are seeing some side effect of that. Look at the hexdump below - it's perfectly clear that there are 21 lines and that the last character in line 21 is \n. All Drupal core files end with one and only one \n, no Drupal core has an "extra one blank line". Adding extra blank lines at the end of module files, for example, can lead to the dreaded "Cannot modify header information - headers already sent" error, which is why this rule is part of the Drupal coding standards.

0000  3c 3f 70 68 70 0a 0a 2f  2a 2a 0a 20 2a 20 40 66  |<?php../**. * @f|
0010  69 6c 65 0a 20 2a 20 54  68 65 20 50 48 50 20 70  |ile. * The PHP p|
0020  61 67 65 20 74 68 61 74  20 73 65 72 76 65 73 20  |age that serves |
0030  61 6c 6c 20 70 61 67 65  20 72 65 71 75 65 73 74  |all page request|
0040  73 20 6f 6e 20 61 20 44  72 75 70 61 6c 20 69 6e  |s on a Drupal in|
0050  73 74 61 6c 6c 61 74 69  6f 6e 2e 0a 20 2a 0a 20  |stallation.. *. |
0060  2a 20 54 68 65 20 72 6f  75 74 69 6e 65 73 20 68  |* The routines h|
0070  65 72 65 20 64 69 73 70  61 74 63 68 20 63 6f 6e  |ere dispatch con|
0080  74 72 6f 6c 20 74 6f 20  74 68 65 20 61 70 70 72  |trol to the appr|
0090  6f 70 72 69 61 74 65 20  68 61 6e 64 6c 65 72 2c  |opriate handler,|
00a0  20 77 68 69 63 68 20 74  68 65 6e 0a 20 2a 20 70  | which then. * p|
00b0  72 69 6e 74 73 20 74 68  65 20 61 70 70 72 6f 70  |rints the approp|
00c0  72 69 61 74 65 20 70 61  67 65 2e 0a 20 2a 0a 20  |riate page.. *. |
00d0  2a 20 41 6c 6c 20 44 72  75 70 61 6c 20 63 6f 64  |* All Drupal cod|
00e0  65 20 69 73 20 72 65 6c  65 61 73 65 64 20 75 6e  |e is released un|
00f0  64 65 72 20 74 68 65 20  47 4e 55 20 47 65 6e 65  |der the GNU Gene|
0100  72 61 6c 20 50 75 62 6c  69 63 20 4c 69 63 65 6e  |ral Public Licen|
0110  73 65 2e 0a 20 2a 20 53  65 65 20 43 4f 50 59 52  |se.. * See COPYR|
0120  49 47 48 54 2e 74 78 74  20 61 6e 64 20 4c 49 43  |IGHT.txt and LIC|
0130  45 4e 53 45 2e 74 78 74  2e 0a 20 2a 2f 0a 0a 2f  |ENSE.txt.. */../|
0140  2a 2a 0a 20 2a 20 52 6f  6f 74 20 64 69 72 65 63  |**. * Root direc|
0150  74 6f 72 79 20 6f 66 20  44 72 75 70 61 6c 20 69  |tory of Drupal i|
0160  6e 73 74 61 6c 6c 61 74  69 6f 6e 2e 0a 20 2a 2f  |nstallation.. */|
0170  0a 64 65 66 69 6e 65 28  27 44 52 55 50 41 4c 5f  |.define('DRUPAL_|
0180  52 4f 4f 54 27 2c 20 67  65 74 63 77 64 28 29 29  |ROOT', getcwd())|
0190  3b 0a 0a 72 65 71 75 69  72 65 5f 6f 6e 63 65 20  |;..require_once |
01a0  44 52 55 50 41 4c 5f 52  4f 4f 54 20 2e 20 27 2f  |DRUPAL_ROOT . '/|
01b0  69 6e 63 6c 75 64 65 73  2f 62 6f 6f 74 73 74 72  |includes/bootstr|
01c0  61 70 2e 69 6e 63 27 3b  0a 64 72 75 70 61 6c 5f  |ap.inc';.drupal_|
01d0  62 6f 6f 74 73 74 72 61  70 28 44 52 55 50 41 4c  |bootstrap(DRUPAL|
01e0  5f 42 4f 4f 54 53 54 52  41 50 5f 46 55 4c 4c 29  |_BOOTSTRAP_FULL)|
01f0  3b 0a 6d 65 6e 75 5f 65  78 65 63 75 74 65 5f 61  |;.menu_execute_a|
0200  63 74 69 76 65 5f 68 61  6e 64 6c 65 72 28 29 3b  |ctive_handler();|
0210  0a                                                |.|

<tr>.
earlofsandwich’s picture

earlofsandwich commented

You're right, I was describing a newline as a separator between two lines (albeit a null one) rather than a line terminator. I see that \n as a line terminator is the more common use, (thanks Wikipedia) and therefore index.php would have 21 actual lines rather than 22. Apologies if I added to the mire of confusion there!

I see, TR, that you have updated the PAReview.sh issue queue. Hopefully this will stop all the misunderstandings and we can all get our modules approved again - I'm pretty new to submitting modules to Drupal so didn't want to wade in to PAReviews issue queue without getting the facts straight.

I suspect it would make sense to change the Drupal documentation to say

All files should end in a single newline character ('\n') - then there would be no ambiguity?

Thanks again for all your help.