Skip to content
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

Print trivia of SynMatchClause #1357

Merged

Conversation

HumbertoCortes
Copy link
Contributor

The comments inside with block (of a try-with) are formatted. In the Clauses generation method ("genClause") a genTrivia call ("genTriviaMainNodesBeforeClausePipe") is inserted in the CodePrinter. The new method look for a trivia nodes with in TriviaMainNodes list.

Two unit test are added to test the correct behaviour.

@knocte
Copy link
Contributor

knocte commented Jan 11, 2021

@nojaf CI on 1st commit was broken because Humberto forgot to run fantomas on fantomas itself, but 2nd commit should have passed, however, the failure seems unrelated to this PR (something about paket...?).

Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

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

Hello @HumbertoCortes, thank you for this contribution.
Unfortunately, I'd like to see a different implementation.
Please let me know if you need further assistance.

src/Fantomas.Tests/TriviaTests.fs Outdated Show resolved Hide resolved
src/Fantomas/CodePrinter.fs Outdated Show resolved Hide resolved
src/Fantomas/Context.fs Outdated Show resolved Hide resolved
@HumbertoCortes HumbertoCortes requested a review from nojaf January 13, 2021 19:22
Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

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

Hey @HumbertoCortes, looks good.
Thanks for the requested changes.
I've tweaked some stuff here and there to work with the full range of SynMatchClause.
Related dotnet/fsharp#10877

@nojaf nojaf changed the title #1219 Solved bug 1219 Print trivia of SynMatchClause Jan 15, 2021
@nojaf
Copy link
Contributor

nojaf commented Jan 15, 2021

Fixes #1219

@nojaf nojaf merged commit f852d7f into fsprojects:master Jan 15, 2021
@HumbertoCortes HumbertoCortes deleted the 1219_SwallowsCommentInsideWithBlock branch January 19, 2021 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants