-
Notifications
You must be signed in to change notification settings - Fork 224
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
Allow Optional Braces Around Template Bodies anyway #3096
Allow Optional Braces Around Template Bodies anyway #3096
Conversation
if (nextIndented) | ||
indentedOnOpen(templateStatSeq(enumCaseAllowed, secondaryConstructorAllowed)) | ||
else if (!enumCaseAllowed && isEndMarkerIntro(tokenPos)) | ||
if (!enumCaseAllowed && isEndMarkerIntro(tokenPos) && !nextIndented) | ||
(selfEmpty(), Nil) | ||
else | ||
syntaxError("expected template body", token) | ||
else | ||
// https://docs.scala-lang.org/scala3/reference/other-new-features/indentation.html#optional-braces-around-template-bodies | ||
// https://github.com/scalameta/scalameta/issues/3093 | ||
indentedOnOpen(templateStatSeq(enumCaseAllowed, secondaryConstructorAllowed)) |
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.
indentedOnOpen
may be called only if it's definitely known that the current token is Indentation.Indent
, and in this case it isn't.
I'd change to this:
if (enumCaseAllowed)
in.observeIndentedEnum()
else
in.observeIndented()
if (acceptOpt[Indentation.Indent])
indentedAfterOpen(templateStatSeq(enumCaseAllowed, secondaryConstructorAllowed))
and the rest could remain unchanged (else if (!enumCaseAllowed...
).
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.
Thanks! that makes sense 👍
95d9c98
to
4b52d3d
Compare
4b52d3d
to
473677d
Compare
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.
one minor comment
@@ -4181,8 +4181,8 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser => | |||
else | |||
in.observeIndented() | |||
|
|||
if (nextIndented) |
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.
the nextIndented variable is no longer used.
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.
Oops, right, thanks!
fix scalameta#3093 previously, scalameta didn't accept the significant indent on anonymous class if it's enclosed by the block with braces. ```scala if (x) { new C: def foo = ??? } ``` Now scalameta accepts the significant indent with `:`.
473677d
to
8108fb0
Compare
fix #3093
It seems like we should accept the optional braces around template body when we accept
:
, even inside the block with braces.I'm trying to confirm this is valid change from syntax specification https://docs.scala-lang.org/scala3/reference/other-new-features/indentation.html (that's why it's still draft)...