Skip to content

Conversation

@dawedawe
Copy link
Member

Fixes #3145

It's actually about type decls without an equals but a comment after.
See here

Ideally we could detect that in expressionExceedsPageWidth (or even below that) as the info.ConfirmedMultiline is the root cause, but I don't see a good way to pass that down there.

@dawedawe dawedawe requested a review from Copilot May 17, 2025 00:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an idempotency issue where a type declaration without an = but with a trailing comment would always emit an extra newline.

  • Introduces a new formatter function that omits the newline in this specific case
  • Updates genTypeDefn to pick newline vs. no-newline behavior based on presence of EqualsToken
  • Adds a targeted test and updates the changelog entry

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Fantomas.Core/Context.fsi Added signature for sepSpaceOrIndentIfExpressionExceedsPageWidth
src/Fantomas.Core/Context.fs Implemented new separator and wired it into expression logic
src/Fantomas.Core/CodePrinter.fs Updated genTypeDefn to select between the two separator functions
src/Fantomas.Core.Tests/TypeDeclarationTests.fs Added test for type decl without = and trailing comment
CHANGELOG.md Documented the fix under Fixed
Comments suppressed due to low confidence (2)

src/Fantomas.Core.Tests/TypeDeclarationTests.fs:3686

  • Add a complementary test for a type alias with an = and trailing comment to ensure the original newline logic (sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidth) remains correct.
let ``type without body but with comment produces superfluous newline, 3145`` () =

src/Fantomas.Core/Context.fs:751

  • [nitpick] Consider renaming this function to something like sepSpaceOrIndentWithoutNlnIfExpressionExceedsPageWidth to more clearly indicate that it omits the newline behavior.
let sepSpaceOrIndentIfExpressionExceedsPageWidth expr (ctx: Context) =

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.

Great, thank you!

@nojaf
Copy link
Contributor

nojaf commented May 20, 2025

@dawedawe do we want to release a 7.0.2?

@dawedawe
Copy link
Member Author

@dawedawe do we want to release a 7.0.2?

I'll look into one or two more fixes in the next couple of days. Let's release after that.

@dawedawe dawedawe merged commit e411e39 into fsprojects:main May 20, 2025
11 of 13 checks passed
@dawedawe dawedawe deleted the fix_3145 branch May 20, 2025 06:13
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.

New lines are added after comment in measure type

2 participants