Lucas-C/pre-commit-hooks

Trailing newline handling in insert-license

Closed this issue · 2 comments

mjpieters commented

Currently there is a rather annoying issue in how a LICENCE.txt file with a trailing newline is handled.

Our LICENSE.txt contains, in abstract, with newlines made explicit:

line1\n
line2\n
line3\n

so the last line ends in a newline. Nothing unusual here.

But this causes the hook to want to insert license headers twice when using a three-element comment style header, like /*| *| */.

That's because the above LICENSE.txt content is then transformed to the prefixed_license list:

['/*\n',
 ' * line1\n',
 ' * line2\n',
 ' * line3\n',
 '\n */']

Note the \n character on the last line. The first time you add a licence header things go sort-of fine, and the files end up with

/*\n
 * line1\n
 * line2\n
 * line3\n
\n
 */\n

File contents start here

Note the extra, empty newline in the header. That by itself is a little irksome, we didn't need an extra newline there.

But next time such a file is modified at the commit hook runs again, you now can't detect that the header is already present, because now the find_license_header_index() function fails to account for the extra newline. There isn't an extra element in the prefixed_license for that line, it's a prefixed newline in the wrong place, the start of an entry.

That's because the source lines \n (the empty line inserted by the script), and \n #} (the line in the prefixed_license list) can never line up. The newline exists in prefixed_license, but not on its own.

The work-around is to commit a LICENSE.txt file with no trailing newline.

A proper fix would add the newline to the last line of the license lines only if there isn't one there already, rather than prefix comment_end. comment_end itself also needs a newline character though:

if not prefixed_license[-1].endswith(eol):
    prefixed_license[-1] += eol  # make sure the license text ends in a newline
if comment_start:
    prefixed_license = [comment_start + eol] + prefixed_license
if comment_end:
    prefixed_license = prefixed_license + [comment_end + eol]
Lucas-C commented

Thank you very much for spotting this bug and providing a patch !

I'm currently on a trip and won't be able to make a release before a month.
Don't hesitate to ping me then if ever I forget about this ticket.