-
Notifications
You must be signed in to change notification settings - Fork 54
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
Add inferred type name hints #118
base: net211
Are you sure you want to change the base?
Conversation
This is ready for review now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here're a few nits and a request to check for null checks in the tree visitor. The rest looks good to me!
ReSharper.FSharp/src/FSharp.Psi.Features/src/Daemon/Stages/InferredTypeHintStage.fs
Outdated
Show resolved
Hide resolved
ReSharper.FSharp/src/FSharp.Psi.Features/src/Daemon/Stages/InferredTypeHintStage.fs
Outdated
Show resolved
Hide resolved
ReSharper.FSharp/src/FSharp.Psi.Features/src/Daemon/Stages/InferredTypeHintStage.fs
Outdated
Show resolved
Hide resolved
ReSharper.FSharp/src/FSharp.Psi.Features/src/Daemon/Stages/InferredTypeHintStage.fs
Outdated
Show resolved
Hide resolved
@saul I've fixed the remaining tests. For some reason turning these hints off doesn't work for me, could you take a look? |
Bump @saul : us F#ers need this feature! 😄 |
@saul @auduchinok : Where are we on this feature? |
…pat-hints # Conflicts: # ReSharper.FSharp/src/FSharp.Common/src/Settings/FSharpOptions.fs
Rebased onto net203, tests working and tested manually. This should be good to go @auduchinok :) |
@auduchinok @saul Can this be reviewed please? Looks very neat :) |
Especially now with ctrl to toggle inline hints. Inline hints arent as intrusive as before since you only see them when needed. |
@drhumlen Exactly! I would like to put inline signatures to toggle behaviour too. This way we could have very clean code with help only when needed |
@saul @auduchinok Hey! Can we do something to make this feature green and merged? |
@En3Tho Yes, will soon look into it. |
Are there any updates on this? This PR has been open for 3 years. Is there something that the rest of the community can do to get this merged? |
@nathanpovo Thanks for writing about it! We're planning to pick it up soon. |
Todo: