Skip to content

Fixed lineBreak for arrowFunction#336

Closed
AndreasHoffmann2 wants to merge 1 commit into
trullock:masterfrom
froebel:fixArrowFunctionLineBreak
Closed

Fixed lineBreak for arrowFunction#336
AndreasHoffmann2 wants to merge 1 commit into
trullock:masterfrom
froebel:fixArrowFunctionLineBreak

Conversation

@AndreasHoffmann2
Copy link
Copy Markdown

@AndreasHoffmann2 AndreasHoffmann2 commented Aug 19, 2022

We ran into a syntax-error when using an arrow-function:
MicrosoftTeams-image
The line-break before "=>" should not be there.

Created a unit-test and fixed the issue.

Comment on lines +3988 to +3995
if (node.FunctionType == FunctionType.ArrowFunction)
{
Output(')');
}
else
{
OutputPossibleLineBreak(')');
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is needed?

Copy link
Copy Markdown
Author

@AndreasHoffmann2 AndreasHoffmann2 Aug 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very sure: If you generate the code without this, you will get a compile-error for the generated code.
But I don't like the lack of elegance in this change either. I thought about creating a new overload for Output: Output(char, bool allowLineBreak). But I did not want to be too intrusive and had no better idea.

In fact, I was unable to reach the code in line 4003 below with the unit-tests. But I left the change there anyway, since it seems to be the better choice.

@AndreasHoffmann2
Copy link
Copy Markdown
Author

We found several other issues with the line-breaks and are currently working on a generic test and the necessary fixes. Please do not merge this one for the moment.

@AndreasHoffmann2
Copy link
Copy Markdown
Author

Closed since we have a better test now: #338

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants