Looks like argumentIndex is evaluated to 0 when typing a comma after the first argument, which is the cause of the second two screenshots. If I force it to be 1, it highlights the correct parameter, so that seems to be the source of the error. I also noticed that it will sometimes evaluate to 1 correctly, but only after tying the whole expression, deleting up until the comma, clicking there, deleting the comma, and then pressing , again.

In short, I think it's buggy.

Yeah, there's a -1 on the argument index which may be going awry. It's there for.the case where "(" is the start of a zero-argument list "()". We can report the argument index in the package of data handed off to unit testing.

I guess I should clarify - I believe that the right behavior here is the one that C# does (and that this PR does as well). Showing all parameters could be too much on the eyes for any method with more than 3 parameters. I don't have data proving this, but I believe that there are enough cases out there that justify this.

It's tricky. We've shown all the parameters for a long time, so I'm used to it. Also (and I think crucially), VIsual F# just doesn't have the same full set of navigational/library-lookup mechanisms as C#, so this is the only way an F# user gets information, short of looking up the docs.

We went around a similar loop with QuickInfo displays for VF# 2.0. F# shows much more info than C#. We thought about changing to C#'s version, but soon discovered that you could just no longer find out anything from F#.

So my gut feeling is that we should show at least what we showed in VS2008-15, which is all the parameters. Showing less is prettier, but leaves the user in a very awkward place if they want to get more information. At the moment even F1 help to go to MSDN is not wired up.

@dsyme Okay, How about I revert the call the BuildMethodOverloadTipText with false and create an issue with what I think the behavior should be.

I think that the current behavior (hovering over something only provides the summary, invoking that QuickInfo item with (, <, ,) should be flipped. I can provide some examples of what that looks like.

ok

@dsyme I think I figured out the argument index problem.

It works as expected for all of these. Deleting and re-adding a comma or open paren also works:

type Foo() =
    /// <summary>
    /// Does even more stuff.
    /// </summary>
    member this.DoAnotherThing(aString, anInt, aFloat) =
        sprintf "The string is: %s, and the int is: %d, and the float is: %f" aString anInt aFloat

    /// <summary>
    /// Does other stuff with lots of parameters.  It really is a lot of stuff and this is a fairly long summary comment.
    /// </summary>
    /// <param name="aString">This is the string which gets passed into this method.  This is kind of a long param comment, but not uncommon.</param>
    /// <param name="anInt">This is the integer which gets passed into this method.  This is kind of a long param comment, but not uncommon.</param>
    /// <param name="aFloat">This is the floating-point value which gets passed into this method.  This is kind of a long param comment, but not uncommon.</param>
    /// <param name="anArray">This is the array which gets passed into this method.  This is kind of a long param comment, but not uncommon.</param>
    /// <exception cref="System.Exception">Thrown when the exception is thrown.</exception>
    /// <exception cref="System.ArgumentException">Thrown when the exception is thrown.</exception>
    /// <exception cref="System.ArgumentNullException">Thrown when the exception is thrown.</exception>
    /// <exception cref="System.Aggregate">Thrown when the exception is thrown.</exception>
    member this.HasManyParameters(aString, anInt, aFloat, anArray: 'a[]) =
        sprintf "The string is: %s, and the int is: %d, and the float is: %f, and the array is: %A" aString anInt aFloat anArray
    
    /// <summary>
    /// Does some stuff.
    /// </summary>
    member this.DoStuff(aString, anInt) =
        sprintf "The string is: %s, and the int is: %d" aString anInt

    /// <summary>
    /// Does other stuff.
    /// </summary>
    /// <param name="aString">A string!</param>
    /// <param name="anInt">An int!</param>
    member this.DoOtherStuff(aString, anInt) =
        sprintf "The string is: %s, and the int is: %d" aString anInt

It looks like the unit tests need updating

1) Failed : Microsoft.VisualStudio.FSharp.Editor.Tests.Roslyn.SignatureHelpProvider.ShouldGiveSignatureHelpAtCorrectMarkers
FSharpCompletionProvider.ProvideMethodsAsyncAux() gave unexpected results, expected Some ("[7..40)", 0, 2, None), got Some ("[7..40)", 1, 2, Some "arg1")

If you need to, follow the instructions at the top of the Signature Help test file to run the tests in F# Interactive and dump the update expected results.

Fix looks good otherwise!!

Ah! I'll update that test (and perhaps add a few more test cases).

@cartermp Great! :)

I modified the first test case a bit and added another expected clause. The rest were just adjustments to the expected span which comes back from ProvideMethodsAsyncAux.

Close/reopen to run tests again

@cartermp does this PR fix the following duplication?

@vasily-kirichenko Technically yes, but it's still not that great of a solution. The best we can do is to change this line to pass in [||] instead of doc, and we won't get Roslyn-based parameter info at the bottom. However, the active parameter still won't be highlighted. I'm inclined not to do this because I prefer duplication over not highlighting the active parameter.

Alternatively, we would request a Roslyn feature to understand the way that we currently display all parameters, and highlight the active one based on that. I think it's very unlikely that such a change would be accepted at this time.

@dsyme @vasily-kirichenko

We should remove the duplicated line and fix f1 and f12 to allow us to show the additional information.
@cartertmp Please remove the duplicated information and ensure there is an issue trcking the extra work.

Kevin

@cartermp can you prepare a PR to just remove the redundent line. I will pull this as is.