-
Notifications
You must be signed in to change notification settings - Fork 822
Trigger add open for FS0043 #2474
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
Conversation
Pretty cool :) |
Did you intend to change System.ValueTuple.dll in this PR? |
@smoothdeveloper No...hmmm. Maybe that's because I ran |
Rebased that change away. I think I used |
@@ -133,6 +133,7 @@ type internal FSharpAddOpenCodeFixProvider | |||
|
|||
let quilifySymbolFixes = | |||
candidates | |||
|> Seq.filter (fun (entity,_) -> not(entity.FullRelativeName.Contains("op_"))) // Don't include fully-qualified operator names. The resultant suggestion makes the code not compile. |
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.
maybe StartsWith? Is there another way we can verify this is an actual operator?
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.
Modified. I'm not aware of another way (although I imagine there's something in the language service which we could use).
@@ -133,6 +133,7 @@ type internal FSharpAddOpenCodeFixProvider | |||
|
|||
let quilifySymbolFixes = |
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.
time to fix that typo :)
@@ -133,6 +133,7 @@ type internal FSharpAddOpenCodeFixProvider | |||
|
|||
let quilifySymbolFixes = |
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.
time to fix that typo :)
omg I'm bad at git and comitted the freaking binary again |
candidates | ||
|> Seq.filter (fun (entity,_) -> not(entity.FullRelativeName.StartsWith("op_"))) // Don't include qualified operator names. The resultant codefix won't compile because it won't be an infix operator anymore. |
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.
FullRelativeName can be "M.N.op_xxx", so this filtering won't work for those.
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.
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.
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.
👍, thanks.
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.
fixed: AddOpenCodeFixProvider may suggest name qualifying for operators
It works perfectly thanks to @vasily-kirichenko's commit. Ready for merge! |
* Trigger add open for FS0043 * Contains -> StartsWith * fixed: AddOpenCodeFixProvider may suggest name qualifying for operators
I'm not sure if this is a good solution, but this does fix #2473. I can now do a full Suave app by using codefixes with this change: