Issue 9288070: Don't allow escaping newlines in single line strings.

Issue 9288070: Don't allow escaping newlines in single line strings. (Closed)

Created:
8 years, 11 months ago by ahe

Modified:
8 years, 11 months ago

Reviewers:
Lasse Reichstein Nielsen

CC:
reviews_dartlang.org, kasperl, floitsch, karlklose, ngeoffray

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge

Visibility:
Public.

More Reviews

Description

Don't allow escaping newlines in single line strings. Committed: https://code.google.com/p/dart/source/detail?r=3624

Patch Set 2 : Address review comments. #

Created: 8 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+87 lines, -38 lines)			Patch
M	dart/frog/leg/scanner/scanner.dart	View		2 chunks	+18 lines, -20 lines	0 comments	Download
M	dart/frog/leg/scanner/source_list.dart	View		8 chunks	+1 line, -18 lines	0 comments	Download
M	dart/tests/language/language.status	View		3 chunks	+13 lines, -0 lines	0 comments	Download
A	dart/tests/language/src/StringEscape1NegativeTest.dart	View		1 chunk	+11 lines, -0 lines	0 comments	Download
A	dart/tests/language/src/StringEscape2NegativeTest.dart	View		1 chunk	+11 lines, -0 lines	0 comments	Download
A	dart/tests/language/src/StringEscape2NegativeTestHelper.dart	View	1	1 chunk	+5 lines, -0 lines	0 comments	Download
A	dart/tests/language/src/StringEscape3NegativeTest.dart	View		1 chunk	+11 lines, -0 lines	0 comments	Download
A	dart/tests/language/src/StringEscape3NegativeTestHelper.dart	View	1	1 chunk	+7 lines, -0 lines	0 comments	Download
A	dart/tests/language/src/StringEscape4NegativeTest.dart	View		1 chunk	+10 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Lasse Reichstein Nielsen

LGTM https://chromiumcodereview.appspot.com/9288070/diff/1/dart/frog/leg/scanner/scanner.dart File dart/frog/leg/scanner/scanner.dart (right): https://chromiumcodereview.appspot.com/9288070/diff/1/dart/frog/leg/scanner/scanner.dart#newcode668 dart/frog/leg/scanner/scanner.dart:668: appendByteStringToken(STRING_INFO, utf8String(start, -2)); Consider moving this line and ...

8 years, 11 months ago (2012-01-26 19:19:18 UTC) #2

ahe

8 years, 11 months ago (2012-01-26 19:30:27 UTC) #3

Thank you, Lasse.

https://chromiumcodereview.appspot.com/9288070/diff/1/dart/frog/leg/scanner/s...
File dart/frog/leg/scanner/scanner.dart (right):

https://chromiumcodereview.appspot.com/9288070/diff/1/dart/frog/leg/scanner/s...
dart/frog/leg/scanner/scanner.dart:668: appendByteStringToken(STRING_INFO,
utf8String(start, -2));
On 2012/01/26 19:19:19, Lasse Reichstein Nielsen wrote:
> Consider moving this line and the identical one from ..Identifier to the above
> method.
> Maybe even the appendBeginGroup call.

Good suggestion, I'll look into that later. I don't want to rerun benchmarks :-)

https://chromiumcodereview.appspot.com/9288070/diff/1/dart/tests/language/src...
File dart/tests/language/src/StringEscape2NegativeTestHelper.dart (right):

https://chromiumcodereview.appspot.com/9288070/diff/1/dart/tests/language/src...
dart/tests/language/src/StringEscape2NegativeTestHelper.dart:5: // An empty
file.
On 2012/01/26 19:19:19, Lasse Reichstein Nielsen wrote:
> What are we testing here?

I'll add a comment explaining this is used by another file.

https://chromiumcodereview.appspot.com/9288070/diff/1/dart/tests/language/src...
File dart/tests/language/src/StringEscape3NegativeTestHelper.dart (right):

https://chromiumcodereview.appspot.com/9288070/diff/1/dart/tests/language/src...
dart/tests/language/src/StringEscape3NegativeTestHelper.dart:5: // An empty
library.
On 2012/01/26 19:19:19, Lasse Reichstein Nielsen wrote:
> I can see that. Say what we are testing (i.e., how it can fail).

Ditto.

Expand Messages | Collapse Messages