sdfmt: Explicitly parse shortened function syntax #380
Conversation
maxhaton
force-pushed
the
shortenedfn
branch
from
53df1a8 to
817c2a2
Compare
src/format/parser.d
Outdated
| @@ -2503,6 +2511,18 @@ private: | |||
| assert(0); | |||
| } | |||
|
|
|||
| void parseShortenedFunctionBody() in(match(TokenType.FatArrow)) { | |||
| auto spanGuard = span!Span(); | |||
There was a problem hiding this comment.
Why do you not splice it?
There was a problem hiding this comment.
I changed it, not entirely sure if I know what the difference is precisely?
There was a problem hiding this comment.
Dump the IR, but long story short, the span is extended leftward in a way that fits the hierarchy.
There was a problem hiding this comment.
Also, if there is already a span that does that, then maybe it's not useful to add one more, but I don't remember what is the generated IR here.
src/format/parser.d
Outdated
| space(); | ||
| parseExpression(); | ||
| split(); | ||
| nextToken(); |
There was a problem hiding this comment.
You might want to check it is the token you expect.
There was a problem hiding this comment.
What should it do if it's not? Nothing, then return?
| int _______________________(int x) | ||
| => x + 1; |
There was a problem hiding this comment.
That's good. We also need a test case where the expression in there splits.
There was a problem hiding this comment.
Added something that breaks.
There was a problem hiding this comment.
Yes, this looks good. One thing I have though of since is that we maybe break after the type, in which case I think we do not indent. The way the span are laid out might break this, so it's a good idea to check.
maxhaton
force-pushed
the
shortenedfn
branch
from
817c2a2 to
7920bbf
Compare
| nextToken(); | ||
| space(); | ||
| parseExpression(); | ||
| if (match(TokenType.Semicolon)) { |
There was a problem hiding this comment.
There is a function that does that.
This goes some way to Fix snazzy-d#359, but the result could probably be improved somewhat with a clever span somewhere. This feature is presumably only used for things with fairly short names so one imagines the line wrapping behaviour won't be noticed much anyway...
maxhaton
force-pushed
the
shortenedfn
branch
from
7920bbf to
bbf6406
Compare
| @@ -2503,6 +2511,19 @@ private: | |||
| assert(0); | |||
| } | |||
|
|
|||
| void parseShortenedFunctionBody() in(match(TokenType.FatArrow)) { | |||
| auto spanGuard = spliceSpan(); | |||
There was a problem hiding this comment.
Does it change something if you remove this? There might be a span here already, but if there isn't this is good to have.
deadalnix
force-pushed
the
master
branch
3 times, most recently
from
0896895 to
5a79292
Compare
deadalnix
force-pushed
the
master
branch
3 times, most recently
from
7ce40a5 to
4dbe602
Compare
This goes some way to Fix #359, but the result could probably be improved somewhat with a clever span somewhere.
This feature is presumably only used for things with fairly short names so one imagines the line wrapping behaviour won't be noticed much anyway...