Skip to content

Commit

Permalink
Don't show empty parameter XML Doc in signature helper and correctly …
Browse files Browse the repository at this point in the history 
…compute argument index (#1895)

* Show 'no documentation' as QuickInfo workaround when no XML doc comments are defined

* Don't add newline if there aren't parameter docs

* Don't ask BuildMethodOverloadTipText to also handle parameters

* Show all parameters when invoking signature helper

* Account for edge case when computing argument index.

* Adjust tests
  • Loading branch information
@cartermp @KevinRansom
cartermp authored and KevinRansom committed Dec 6, 2016
1 parent 6826756 commit 50e7a82
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 47 deletions.
30 changes: 23 additions & 7 deletions vsintegration/src/FSharp.Editor/SignatureHelp.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,23 @@ type internal FSharpSignatureHelpProvider
return None // comma or paren at wrong location = remove help display
| _ ->

// Compute the argument index by working out where the caret is between the various commas
// Compute the argument index by working out where the caret is between the various commas.
let argumentIndex =
tupleEnds
|> Array.pairwise
|> Array.tryFindIndex (fun (lp1,lp2) -> textLines.GetTextSpan(LinePositionSpan(lp1, lp2)).Contains(caretPosition))
|> (function None -> 0 | Some n -> n)
let computedTextSpans =
tupleEnds
|> Array.pairwise
|> Array.map (fun (lp1, lp2) -> textLines.GetTextSpan(LinePositionSpan(lp1, lp2)))

match (computedTextSpans|> Array.tryFindIndex (fun t -> t.Contains(caretPosition))) with
| None ->
// Because 'TextSpan.Contains' only succeeeds if 'TextSpan.Start <= caretPosition < TextSpan.End' is true,
// we need to check if the caret is at the very last position in the TextSpan.
//
// We default to 0, which is the first argument, if the caret position was nowhere to be found.
if computedTextSpans.[computedTextSpans.Length-1].End = caretPosition then
computedTextSpans.Length-1
else 0
| Some n -> n

// Compute the overall argument count
let argumentCount =
Expand All @@ -168,10 +179,15 @@ type internal FSharpSignatureHelpProvider
[| for p in parameters do
// FSROSLYNTODO: compute the proper help text for parameters, c.f. AppendParameter in XmlDocumentation.fs
let paramDoc = XmlDocumentation.BuildMethodParamText(documentationBuilder, method.XmlDoc, p.ParameterName)
let doc = [| TaggedText(TextTags.Text, paramDoc); |]
let doc = if String.IsNullOrWhiteSpace(paramDoc) then [||]
else [| TaggedText(TextTags.Text, paramDoc) |]
yield (p.ParameterName,p.IsOptional,doc,[| TaggedText(TextTags.Text,p.Display) |]) |]

let doc = [| TaggedText(TextTags.Text, methodDocs + "\n") |]
let hasParamComments (pcs: (string*bool*TaggedText[]*TaggedText[])[]) =
pcs |> Array.exists (fun (_, _, doc, _) -> doc.Length > 0)

let doc = if (hasParamComments parameters) then [| TaggedText(TextTags.Text, methodDocs + "\n") |]
else [| TaggedText(TextTags.Text, methodDocs) |]

// Prepare the text to display
let descriptionParts = [| TaggedText(TextTags.Text, method.TypeText) |]
Expand Down
80 changes: 40 additions & 40 deletions vsintegration/tests/unittests/SignatureHelpProviderTests.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,28 +66,28 @@ let ShouldGiveSignatureHelpAtCorrectMarkers() =
let manyTestCases =
[ ("""
//1
System.Console.WriteLine(1,arg1=2)
System.Console.WriteLine(format="Hello, {0}",arg0="World")
""",
[(".", None);
("System", None);
("WriteLine", None);
("(", Some ("[7..40)", 0, 2, None));
(",", Some ("[7..40)", 1, 2, Some "arg1"));
("arg", Some ("[7..40)", 1, 2, Some "arg1"));
("arg1", Some ("[7..40)", 1, 2, Some "arg1"));
("=", Some ("[7..40)", 1, 2, Some "arg1"));
("2", Some ("[7..40)", 0, 2, None));
("(", Some ("[5..62)", 0, 2, Some "format"));
("format", Some ("[5..62)", 0, 2, Some "format"));
(",", None);
("""",""", Some ("[5..62)", 1, 2, Some "arg0"));
("arg0", Some ("[5..62)", 1, 2, Some "arg0"));
("arg0=", Some ("[5..62)", 1, 2, Some "arg0"));
("World", Some ("[5..62)", 1, 2, Some "arg0"));
(")", None)]);
( """
//2
open System
Console.WriteLine([(1,2)])
""",
[
("WriteLine(", Some ("[20..45)", 0, 0, None));
("WriteLine(", Some ("[17..42)", 0, 0, None));
(",", None);
("[(", Some ("[20..45)", 0, 1, None))
("[(", Some ("[17..42)", 0, 1, None))
]);
( """
//3
Expand All @@ -97,21 +97,21 @@ type foo3 = N1.T<ParamIgnored=
type foo4 = N1.T<Param1=1,
type foo5 = N1.T<Param1=1,ParamIgnored=
""",
[("type foo = N1.T<", Some ("[18..26)", 0, 0, None));
("type foo2 = N1.T<", Some ("[38..52)", 0, 0, Some "Param1"));
("type foo2 = N1.T<Param1", Some ("[38..52)", 0, 1, Some "Param1"));
("type foo2 = N1.T<Param1=", Some ("[38..52)", 0, 1, Some "Param1"));
("type foo3 = N1.T<", Some ("[64..84)", 0, 0, Some "ParamIgnored"));
("type foo3 = N1.T<ParamIgnored=", Some ("[64..84)", 0, 1, Some "ParamIgnored"));
("type foo4 = N1.T<Param1", Some ("[96..112)", 0, 2, Some "Param1"));
("type foo4 = N1.T<Param1=", Some ("[96..112)", 0, 2, Some "Param1"));
("type foo4 = N1.T<Param1=1", Some ("[96..112)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1", Some ("[124..153)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=", Some ("[124..153)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=1", Some ("[124..153)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=1,", Some ("[124..153)", 1, 2, Some "ParamIgnored"));
("type foo5 = N1.T<Param1=1,ParamIgnored",Some ("[124..153)", 1, 2, Some "ParamIgnored"));
("type foo5 = N1.T<Param1=1,ParamIgnored=",Some ("[124..153)", 1, 2, Some "ParamIgnored"))
[("type foo = N1.T<", Some ("[16..23)", 0, 0, None));
("type foo2 = N1.T<", Some ("[35..48)", 0, 0, Some "Param1"));
("type foo2 = N1.T<Param1", Some ("[35..48)", 0, 1, Some "Param1"));
("type foo2 = N1.T<Param1=", Some ("[35..48)", 0, 1, Some "Param1"));
("type foo3 = N1.T<", Some ("[60..79)", 0, 0, Some "ParamIgnored"));
("type foo3 = N1.T<ParamIgnored=", Some ("[60..79)", 0, 1, Some "ParamIgnored"));
("type foo4 = N1.T<Param1", Some ("[91..106)", 0, 2, Some "Param1"));
("type foo4 = N1.T<Param1=", Some ("[91..106)", 0, 2, Some "Param1"));
("type foo4 = N1.T<Param1=1", Some ("[91..106)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1", Some ("[118..146)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=", Some ("[118..146)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=1", Some ("[118..146)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=1,", Some ("[118..146)", 1, 2, Some "ParamIgnored"));
("type foo5 = N1.T<Param1=1,ParamIgnored",Some ("[118..146)", 1, 2, Some "ParamIgnored"));
("type foo5 = N1.T<Param1=1,ParamIgnored=",Some ("[118..146)", 1, 2, Some "ParamIgnored"))
]);
( """
//4
Expand All @@ -121,21 +121,21 @@ type foo3 = N1.T<ParamIgnored= >
type foo4 = N1.T<Param1=1, >
type foo5 = N1.T<Param1=1,ParamIgnored= >
""",
[("type foo = N1.T<", Some ("[18..24)", 0, 0, None));
("type foo2 = N1.T<", Some ("[40..53)", 0, 0, Some "Param1"));
("type foo2 = N1.T<Param1", Some ("[40..53)", 0, 1, Some "Param1"));
("type foo2 = N1.T<Param1=", Some ("[40..53)", 0, 1, Some "Param1"));
("type foo3 = N1.T<", Some ("[68..87)", 0, 0, Some "ParamIgnored"));
("type foo3 = N1.T<ParamIgnored=", Some ("[68..87)", 0, 1, Some "ParamIgnored"));
("type foo4 = N1.T<Param1", Some ("[102..117)", 0, 2, Some "Param1"));
("type foo4 = N1.T<Param1=", Some ("[102..117)", 0, 2, Some "Param1"));
("type foo4 = N1.T<Param1=1", Some ("[102..117)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1", Some ("[132..160)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=", Some ("[132..160)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=1", Some ("[132..160)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=1,", Some ("[132..160)", 1, 2, Some "ParamIgnored"));
("type foo5 = N1.T<Param1=1,ParamIgnored",Some ("[132..160)", 1, 2, Some "ParamIgnored"));
("type foo5 = N1.T<Param1=1,ParamIgnored=",Some ("[132..160)", 1, 2, Some "ParamIgnored"))])
[("type foo = N1.T<", Some ("[16..22)", 0, 0, None));
("type foo2 = N1.T<", Some ("[37..50)", 0, 0, Some "Param1"));
("type foo2 = N1.T<Param1", Some ("[37..50)", 0, 1, Some "Param1"));
("type foo2 = N1.T<Param1=", Some ("[37..50)", 0, 1, Some "Param1"));
("type foo3 = N1.T<", Some ("[64..83)", 0, 0, Some "ParamIgnored"));
("type foo3 = N1.T<ParamIgnored=", Some ("[64..83)", 0, 1, Some "ParamIgnored"));
("type foo4 = N1.T<Param1", Some ("[97..112)", 0, 2, Some "Param1"));
("type foo4 = N1.T<Param1=", Some ("[97..112)", 0, 2, Some "Param1"));
("type foo4 = N1.T<Param1=1", Some ("[97..112)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1", Some ("[126..154)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=", Some ("[126..154)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=1", Some ("[126..154)", 0, 2, Some "Param1"));
("type foo5 = N1.T<Param1=1,", Some ("[126..154)", 1, 2, Some "ParamIgnored"));
("type foo5 = N1.T<Param1=1,ParamIgnored",Some ("[126..154)", 1, 2, Some "ParamIgnored"));
("type foo5 = N1.T<Param1=1,ParamIgnored=",Some ("[126..154)", 1, 2, Some "ParamIgnored"))])
//Test case 5
( """let _ = System.DateTime(""",
[("let _ = System.DateTime(", Some ("[8..24)", 0, 0, None)) ])
Expand Down

0 comments on commit 50e7a82

Please sign in to comment.