Uh oh!
There was an error while loading. Please reload this page.
Fix vs std11 commands - #2645
Conversation
| elif nCmdId = uint32 VSConstants.VSStd11CmdID.ExecuteLineInInteractive then | ||
| Hooks.OnMLSend projectSystemPackage.Value FsiEditorSendAction.ExecuteLine null null | ||
| if pguidCmdGroup = VSConstants.VsStd11 && nCmdId = uint32 VSConstants.VSStd11CmdID.ExecuteSelectionInInteractive then | ||
| do Hooks.OnMLSend projectSystemPackage.Value FsiEditorSendAction.ExecuteSelection null null |
There was a problem hiding this comment.
To answer your question on the issue ticket, the 'do' aren't necessary :)
There was a problem hiding this comment.
I'm brand new to F# so I'm willing to respect whatever coding conventions are in place for this repo. I've go no strong feelings one way or the other so I can remove them if people prefer.
There was a problem hiding this comment.
I can't speak for the actual maintainers of the code base, but idiomatic F# doesn't use 'do'. I'm not familiar with the VFT code base enough to state which is right but I've never seen 'do' used in production code at our work.
There was a problem hiding this comment.
@JoshVarty welcome to F# programming.
Please loose the do, in general match the style of the surrounding code is the watchword. In this case the do is unnecessary.
Kevin
There was a problem hiding this comment.
@JoshVarty just to give some context where do is used:
if you need some initialization code to run in constructor of types:
typeMyType(myParameter)=do
printfn "my parameter is: %s" myParameter
// more code// rest of that typealso, can be useful to scope a disposable to a smaller scope than the function:
letmyFunction()=leta= something ()douse willDisposeSoon =new SomethingDisposable()// ....// dispose called here
doSomethingElse()|> ignore
// some more codeIn most other cases (I've encountered), it is mostly noise.
There was a problem hiding this comment.
👍
Thanks, I'll apply the change.
KevinRansom
left a comment
There was a problem hiding this comment.
Looks Great thanks for this.
| elif nCmdId = uint32 VSConstants.VSStd11CmdID.ExecuteLineInInteractive then | ||
| Hooks.OnMLSend projectSystemPackage.Value FsiEditorSendAction.ExecuteLine null null | ||
| if pguidCmdGroup = VSConstants.VsStd11 && nCmdId = uint32 VSConstants.VSStd11CmdID.ExecuteSelectionInInteractive then | ||
| do Hooks.OnMLSend projectSystemPackage.Value FsiEditorSendAction.ExecuteSelection null null |
There was a problem hiding this comment.
@JoshVarty welcome to F# programming.
Please loose the do, in general match the style of the surrounding code is the watchword. In this case the do is unnecessary.
Kevin
KevinRansom
commented
Mar 18, 2017
Thanks for this mate Kevin |
* Only return VSConstants.S_OK for commands we handle. * Remove do keyword
* Only return VSConstants.S_OK for commands we handle. * Remove do keyword
Fixes#2630 and #2335
The issue is that we were marking all commands as "handled" for
VSConstants.VsStd11andGuids.guidInteractivecommand groups.If we can't handle these commands we'll just let the others in the command chain have a chance to handle them.
Thanks to @majocha for help with the code.