Add a test for trimming leading whitespace in multiline strings

Add a test for trimming leading whitespace in multiline strings

R=paulberry@google.com

Committed: https://code.google.com/p/dart/source/detail?r=44755

[Brian Wilkerson, 2015-03-27 18:47:07]

brianwilkerson@google.com changed reviewers:
+ paulberry@google.com

[Paul Berry, 2015-03-27 19:12:58]

lgtm

[Brian Wilkerson, 2015-03-27 19:28:23]

Committed patchset #1 (id:1) manually as 44755 (presubmit successful).

[hausner, 2015-03-31 22:15:36]

hausner@google.com changed reviewers:
+ hausner@google.com

[hausner, 2015-03-31 22:15:36]

This test is wrong in my opinion.

https://codereview.chromium.org/1042523004/diff/1/tests/language/multiline_st...
File tests/language/multiline_strings_test.dart (right):

https://codereview.chromium.org/1042523004/diff/1/tests/language/multiline_st...
tests/language/multiline_strings_test.dart:11: Expect.equals('\\\nfoo', '''\\
I believe this expectation is wrong. The Spec essentially says that a backslash
in front of a space is swallowed, but not any arbitrary backlsash.

[hausner, 2015-03-31 22:33:05]

Correction.

https://codereview.chromium.org/1042523004/diff/1/tests/language/multiline_st...
File tests/language/multiline_strings_test.dart (right):

https://codereview.chromium.org/1042523004/diff/1/tests/language/multiline_st...
tests/language/multiline_strings_test.dart:11: Expect.equals('\\\nfoo', '''\\
On 2015/03/31 22:15:36, hausner wrote:
> I believe this expectation is wrong. The Spec essentially says that a
backslash
> in front of a space is swallowed, but not any arbitrary backlsash.

My bad. This expectation is right.

https://codereview.chromium.org/1042523004/diff/1/tests/language/multiline_st...
tests/language/multiline_strings_test.dart:15: foo''');
This multiline string should be equal to 'foo', since the first line only
contains whitespace (a tab) before the newline.

[Paul Berry, 2015-03-31 23:33:03]

https://codereview.chromium.org/1042523004/diff/1/tests/language/multiline_st...
File tests/language/multiline_strings_test.dart (right):

https://codereview.chromium.org/1042523004/diff/1/tests/language/multiline_st...
tests/language/multiline_strings_test.dart:15: foo''');
On 2015/03/31 22:33:04, hausner wrote:
> This multiline string should be equal to 'foo', since the first line only
> contains whitespace (a tab) before the newline.

The relevant spec text is: "If the first line of a multiline string consists
solely of the whitespace characters defined by the production WHITESPACE,
possibly prefixed by \, then that line is ignored, including the new line at its
end".

It isn't manifestly stated in the spec text whether it's referring to the raw
uninterpreted characters of the string as they appear in the source file, or the
characters that result after escape sequences have been interpreted.  But we
have to choose one interpretation and use it consistently, and reading between
the lines, I think it's pretty clear that the intention was to refer to the raw
uninterpreted characters.  (If the intention had been to refer to the characters
that result after escape sequences have been interpreted, then there would have
been no need for the phrase "possibly prefixed by \"; in fact, the presence of
this phrase would cause even "\\ " to be trimmed, which matches neither VM nor
Dart2js behavior, and which I think would be very unexpected.)  So under my
interpretation, Brian's test is correct--"\t" should not be trimmed from the
string, because "t" is not a whitespace character.

Should we ask Gilad to weigh in?

[hausner, 2015-04-01 00:13:57]

I spoke to Gilad about this today, but please reassign the bug you filed against
the VM to him and ask for clarification.

[gbracha, 2015-04-02 19:25:57]

gbracha@google.com changed reviewers:
+ gbracha@google.com

[gbracha, 2015-04-02 19:25:58]

Belatedly publishing my comments. Will update bug accordingly.

https://codereview.chromium.org/1042523004/diff/1/tests/language/multiline_st...
File tests/language/multiline_strings_test.dart (right):

https://codereview.chromium.org/1042523004/diff/1/tests/language/multiline_st...
tests/language/multiline_strings_test.dart:15: foo''');
On 2015/03/31 23:33:03, Paul Berry wrote:
> On 2015/03/31 22:33:04, hausner wrote:
> > This multiline string should be equal to 'foo', since the first line only
> > contains whitespace (a tab) before the newline.
> 
> The relevant spec text is: "If the first line of a multiline string consists
> solely of the whitespace characters defined by the production WHITESPACE,
> possibly prefixed by \, then that line is ignored, including the new line at
its
> end".
> 
> It isn't manifestly stated in the spec text whether it's referring to the raw
> uninterpreted characters of the string as they appear in the source file, or
the
> characters that result after escape sequences have been interpreted.  But we
> have to choose one interpretation and use it consistently, and reading between
> the lines, I think it's pretty clear that the intention was to refer to the
raw
> uninterpreted characters.  (If the intention had been to refer to the
characters
> that result after escape sequences have been interpreted, then there would
have
> been no need for the phrase "possibly prefixed by \"; in fact, the presence of
> this phrase would cause even "\\ " to be trimmed, which matches neither VM nor
> Dart2js behavior, and which I think would be very unexpected.)  So under my
> interpretation, Brian's test is correct--"\t" should not be trimmed from the
> string, because "t" is not a whitespace character.
> 
> Should we ask Gilad to weigh in?

The idea was (I believe) to ignore invisible stuff people may have typed after
the opening of the string.  Maybe we should have left it at that, but someone
noted that say, tab and \ followed by tab were the same. But the argument would
apply to \t. It is also much easier to understand that way.

The wording should be clarified, but basically, if it *any* kind of whitespace,
you ignore it. So while Paul and Brian's interpretation is sensible vis-a-vis
the wording, I don't think an ordinary human would expect such subtlety.

In any case, at some point it would make sense to reword the spec. since it
seems insufficient verbiage was dedicated to this weighty topic.