-
-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: show bad location in empty interpolation error #888
test: show bad location in empty interpolation error #888
Conversation
ca62a64
to
f4f814f
Compare
a0865fd
to
b55806d
Compare
@@ -172,15 +172,6 @@ let suites = | |||
Invalid_syntax_of_var " (" ) -> | |||
OUnit.assert_bool __LOC__ true | |||
| _ -> OUnit.assert_bool __LOC__ false ); | |||
( __LOC__ >:: fun _ -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test was duplicated with the one I changed below.
$ melc -ppx melppx x.ml | ||
File "x.ml", line 1, characters 20-23: | ||
1 | let x = {j| Hello, $()|j} | ||
^^^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this on the right location? Seems it's picking ()|
rather than $()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, after reading the title, it's unclear to me if the PR was created to showcase this bad location or to fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh good point, I guess we could move it backwards yet one more char.
The first commit shows the wrong location, and the 2nd commit fixes it.
No description provided.