[webkit-reviews] review denied: [Bug 63340] Lexer error messages are currently appalling : [Attachment 98769] Patch

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has denied  review:
Bug 63340: Lexer error messages are currently appalling
https://bugs.webkit.org/show_bug.cgi?id=63340

Attachment 98769: Patch
https://bugs.webkit.org/attachment.cgi?id=98769&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=98769&action=review

Other than the above comments this looks good.	Once this is done we can move
to JSON error messages or getting line a character information :D

>> Source/JavaScriptCore/parser/Lexer.cpp:531
>> +			m_lexErrorMessage = "\\u can only be followed by a
unicode character number, or a closing quote";
> 
> s/number/sequence i think would be a better wording

s/number/sequence i think would be a better wording

>> Source/JavaScriptCore/parser/Lexer.cpp:569
>> +		    m_lexErrorMessage = "Problem reading character sequence";
> 
> I believe this should be "Unterminated string constant"

I believe this should be "Unterminated string constant"

>> Source/JavaScriptCore/parser/Lexer.cpp:1119
>> +	    m_lexErrorMessage = String::format("Invalid character (\\u%04u)",
m_current).impl();
> 
> I think this should be a function for creating the error message so that
simple characters that are invalid are displayed in a user friendly way, eg:
> `, @, #, \n, \r, \v, \0
> 
> The rest can be shown as a unicode escape

I think this should be a function for creating the error message so that simple
characters that are invalid are displayed in a user friendly way, eg:
`, @, #, \n, \r, \v, \0

The rest can be shown as a unicode escape