Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixed HTML reporter issue with Macintosh End-of-Lines (CR) #138

Merged
merged 2 commits into from Jan 21, 2014

Conversation

Projects
None yet
3 participants
Contributor

asifrc commented Jan 17, 2014

Issue:

One of my projects was failing to generate html reports with the following error:

C:\code\node\buet73\node_modules\istanbul\lib\report\html.js:175
        structuredText[lineNumber].covered = count > 0 ? 'yes' : 'no';
                                                                 ^
TypeError: Cannot set property 'covered' of undefined
    at C:\code\node\buet73\node_modules\istanbul\lib\report\html.js:175:66

I have no idea why, but it turns out my editor somehow was using Macintosh line-endings (\r) instead of the usual Windows(\r\n) or Unix(\n) EOL characters.

Solution:

I replaced the Regular Expression /\r?\n/ in lib/report/html.js with /(\r?\n|\r)/ so that it will return matches for Unix, Windows, and Macintosh line endings when splitting the body of the code into an array of lines.
I additionally added tests for EOL variations to test/cli/test-html-report.js, so that they're accounted for.

I realize that this issue might not be encountered by very many people, but I did happen to come across it and thought that this minor fix might be helpful.

Owner

gotwarlost commented Jan 17, 2014

Thank you for the pull request. Could you please also fix https://github.com/gotwarlost/istanbul/blob/master/lib/instrumenter.js#L593 for consistency.

(It's ok to duplicate the regexp for this purpose instead of getting it from a common place since the instrumenter can be run on the browser as well and needs to be self-contained).

Thanks!

Contributor

asifrc commented Jan 17, 2014

Glad to help :) I modified the regexp and made the change consistent between html.js and instrumenter.js. The regexp from before when used in instrumenter.js was initially causing two tests to fail (which is why I had left it out), but I found that it was caused by using plain parentheses instead of non-matching ones, e.g. (?: ... ). Let me know if there's anything else I can do for this pull request or for istanbul in general :)

I came across the same problem on my Mac.
http://stackoverflow.com/questions/5034781/js-regex-to-split-by-line#comment5633979_5035005
In short, fail proof regex for newline is (\r\n|[\n\v\f\r\x85\u2028\u2029]) according to http://www.unicode.org/reports/tr18/#Line_Boundaries

+1 for this fix!

@gotwarlost gotwarlost added a commit that referenced this pull request Jan 21, 2014

@gotwarlost gotwarlost Merge pull request #138 from asifrc/mac-eol
Fixed HTML reporter issue with Macintosh End-of-Lines (CR)
78d3533

@gotwarlost gotwarlost merged commit 78d3533 into gotwarlost:master Jan 21, 2014

1 check passed

default The Travis CI build passed
Details
Owner

gotwarlost commented Jan 21, 2014

Available in 0.2.4 - thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment