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

Provide a mechanism for pinning type signatures formats during printing #10744

Merged
merged 6 commits into from
Dec 17, 2020

Conversation

baronfel
Copy link
Member

In FSAC we have some pretty convoluted logic for undoing the compiler's default printing of generic type parameters for types that use the suffix form (ie int list).

We'd love a way to explicit ask NicePrint to render the type signature in a given way, either prefix or suffix, regardless of the derived flag on the type itself.

To that end, this PR:

  • introduces a flag on DisplayEnv that represents the rendering mode, defaulting to delegating to the flag on the type,
  • provides a way for an FCS consumer to configure the display environment to prefix or suffix mode, and
  • uses the flag to potentially overwrite the prefix/suffix logic when a type with generic parameters is printed.

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

Formatting changes. Note that NicePrint is also used to print signatures. So some tests might fail because of that and the sig printing would have to be adjusted to match the current behavior.

src/fsharp/TypedTreeOps.fs Outdated Show resolved Hide resolved
src/fsharp/TypedTreeOps.fsi Outdated Show resolved Hide resolved
src/fsharp/TypedTreeOps.fsi Outdated Show resolved Hide resolved
@@ -2796,7 +2802,8 @@ type DisplayEnv =
printVerboseSignatures = false
g = tcGlobals
contextAccessibility = taccessPublic
generatedValueLayout = (fun _ -> None) }
generatedValueLayout = (fun _ -> None)
genericParameterStyle = GenericParameterStyle.Implicit }
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the correct default?

Copy link
Member Author

Choose a reason for hiding this comment

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

This preserves the behavior currently, which is to print based on the state of the flag on the TyCon.

I could see a future FCS release where we say 'nah let's just go for Prefix by default' and we'd change it here, though.

baronfel and others added 2 commits December 16, 2020 16:09
@baronfel
Copy link
Member Author

baronfel commented Dec 16, 2020

I would very much be surprised if any logic tests failed, as I intended to make this feature opt-in entirely. Surface area tests I do expect to fail.

Copy link
Member

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

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

Nice, thank you.

@cartermp cartermp merged commit e83623b into dotnet:main Dec 17, 2020
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
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.

3 participants