From 50e7a829c5ca46976d62866eed2884498dfbfec7 Mon Sep 17 00:00:00 2001 From: Phillip Carter Date: Mon, 5 Dec 2016 16:34:14 -0800 Subject: [PATCH] Don't show empty parameter XML Doc in signature helper and correctly 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 --- .../src/FSharp.Editor/SignatureHelp.fs | 30 +++++-- .../unittests/SignatureHelpProviderTests.fs | 80 +++++++++---------- 2 files changed, 63 insertions(+), 47 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/SignatureHelp.fs b/vsintegration/src/FSharp.Editor/SignatureHelp.fs index 1a9dc1db2c2..f663c877a09 100644 --- a/vsintegration/src/FSharp.Editor/SignatureHelp.fs +++ b/vsintegration/src/FSharp.Editor/SignatureHelp.fs @@ -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 = @@ -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) |] diff --git a/vsintegration/tests/unittests/SignatureHelpProviderTests.fs b/vsintegration/tests/unittests/SignatureHelpProviderTests.fs index c93f658dfd7..670851cb03d 100644 --- a/vsintegration/tests/unittests/SignatureHelpProviderTests.fs +++ b/vsintegration/tests/unittests/SignatureHelpProviderTests.fs @@ -66,18 +66,18 @@ 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 @@ -85,9 +85,9 @@ 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 @@ -97,21 +97,21 @@ type foo3 = N1.T type foo4 = N1.T type foo5 = N1.T """, - [("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