From 913df57cd5689f03e5382eec008324f80c0c9dd8 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 24 Jul 2020 01:21:02 -0700 Subject: [PATCH 1/5] Move omnisharp vscode to the new hover implementation Hover implemention is in PR here: https://github.com/OmniSharp/omnisharp-roslyn/pull/1860. --- src/features/hoverProvider.ts | 42 +++++++++++++++++++++++++---------- src/omnisharp/protocol.ts | 15 +++++++++++++ src/omnisharp/utils.ts | 5 +++++ 3 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/features/hoverProvider.ts b/src/features/hoverProvider.ts index f36dba8ef..4e6561e78 100644 --- a/src/features/hoverProvider.ts +++ b/src/features/hoverProvider.ts @@ -7,26 +7,44 @@ import AbstractSupport from './abstractProvider'; import * as protocol from '../omnisharp/protocol'; import * as serverUtils from '../omnisharp/utils'; import { createRequest } from '../omnisharp/typeConversion'; -import { HoverProvider, Hover, TextDocument, CancellationToken, Position } from 'vscode'; -import { GetDocumentationString } from './documentation'; +import { HoverProvider, Hover, TextDocument, CancellationToken, Position, MarkdownString } from 'vscode'; export default class OmniSharpHoverProvider extends AbstractSupport implements HoverProvider { public async provideHover(document: TextDocument, position: Position, token: CancellationToken): Promise { + let request = createRequest(document, position); + try { + const response = await serverUtils.getQuickInfo(this._server, request, token); + if (!response.Description && !response.RemainingSections && !response.Summary) { + return undefined; + } - let req = createRequest(document, position); - req.IncludeDocumentation = true; + let markdownString = new MarkdownString; + const language = "csharp"; + if (response.Description) { + markdownString.appendCodeblock(response.Description, language); + } - try { - let value = await serverUtils.typeLookup(this._server, req, token); - if (value && value.Type) { - let documentation = GetDocumentationString(value.StructuredDocumentation); - let contents = [{ language: 'csharp', value: value.Type }, documentation]; - return new Hover(contents); + if (response.Summary) { + markdownString.appendMarkdown(response.Summary); } + + if (response.RemainingSections) { + for (const section of response.RemainingSections) { + if (section.IsCSharpCode) { + markdownString.appendCodeblock(section.Text, language); + } + else { + markdownString.appendText("\n"); + markdownString.appendMarkdown(section.Text); + } + } + } + + return new Hover(markdownString); } catch (error) { - return undefined; //No hover result could be obtained + return undefined; } } -} \ No newline at end of file +} diff --git a/src/omnisharp/protocol.ts b/src/omnisharp/protocol.ts index dcf681d0e..cd94adeb1 100644 --- a/src/omnisharp/protocol.ts +++ b/src/omnisharp/protocol.ts @@ -491,6 +491,7 @@ export namespace V2 { export const BlockStructure = '/v2/blockstructure'; export const CodeStructure = '/v2/codestructure'; export const Highlight = '/v2/highlight'; + export const QuickInfo = '/v2/quickinfo'; } export interface SemanticHighlightSpan { @@ -711,6 +712,20 @@ export namespace V2 { Kind: string; } + export interface QuickInfoRequest extends Request { + } + + export interface QuickInfoResponse { + Description?: string; + Summary?: string; + RemainingSections?: QuickInfoResponseSection[]; + } + + export interface QuickInfoResponseSection { + IsCSharpCode: boolean; + Text: string; + } + export module SymbolKinds { // types export const Class = 'class'; diff --git a/src/omnisharp/utils.ts b/src/omnisharp/utils.ts index aed19836c..f4209ccba 100644 --- a/src/omnisharp/utils.ts +++ b/src/omnisharp/utils.ts @@ -9,6 +9,7 @@ import * as path from 'path'; import * as protocol from './protocol'; import * as vscode from 'vscode'; import { MSBuildProject } from './protocol'; +import { CancellationToken } from 'vscode-languageserver-protocol'; export async function autoComplete(server: OmniSharpServer, request: protocol.AutoCompleteRequest, token: vscode.CancellationToken) { return server.makeRequest(protocol.Requests.AutoComplete, request, token); @@ -168,6 +169,10 @@ export async function getSemanticHighlights(server: OmniSharpServer, request: pr return server.makeRequest(protocol.V2.Requests.Highlight, request); } +export async function getQuickInfo(server: OmniSharpServer, request: protocol.V2.QuickInfoRequest, token: CancellationToken) { + return server.makeRequest(protocol.V2.Requests.QuickInfo, request, token); +} + export async function isNetCoreProject(project: protocol.MSBuildProject) { return project.TargetFrameworks.find(tf => tf.ShortName.startsWith('netcoreapp') || tf.ShortName.startsWith('netstandard')) !== undefined; } From d3a14668c13f0563de95b5963ddb964033044cd4 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 24 Jul 2020 16:35:40 -0700 Subject: [PATCH 2/5] Update with the roslyn-side PR. --- src/features/hoverProvider.ts | 28 +++++++++------------------- src/omnisharp/protocol.ts | 29 ++++++++++++++--------------- src/omnisharp/utils.ts | 4 ++-- 3 files changed, 25 insertions(+), 36 deletions(-) diff --git a/src/features/hoverProvider.ts b/src/features/hoverProvider.ts index 4e6561e78..71bc150ad 100644 --- a/src/features/hoverProvider.ts +++ b/src/features/hoverProvider.ts @@ -12,32 +12,22 @@ import { HoverProvider, Hover, TextDocument, CancellationToken, Position, Markdo export default class OmniSharpHoverProvider extends AbstractSupport implements HoverProvider { public async provideHover(document: TextDocument, position: Position, token: CancellationToken): Promise { - let request = createRequest(document, position); + let request = createRequest(document, position); try { const response = await serverUtils.getQuickInfo(this._server, request, token); - if (!response.Description && !response.RemainingSections && !response.Summary) { + if (!response || !response.Sections) { return undefined; } let markdownString = new MarkdownString; const language = "csharp"; - if (response.Description) { - markdownString.appendCodeblock(response.Description, language); - } - - if (response.Summary) { - markdownString.appendMarkdown(response.Summary); - } - - if (response.RemainingSections) { - for (const section of response.RemainingSections) { - if (section.IsCSharpCode) { - markdownString.appendCodeblock(section.Text, language); - } - else { - markdownString.appendText("\n"); - markdownString.appendMarkdown(section.Text); - } + for (const section of response.Sections) { + if (section.IsCSharpCode) { + markdownString.appendCodeblock(section.Text, language); + } + else { + markdownString.appendText("\n"); + markdownString.appendMarkdown(section.Text); } } diff --git a/src/omnisharp/protocol.ts b/src/omnisharp/protocol.ts index cd94adeb1..0ea77e1b0 100644 --- a/src/omnisharp/protocol.ts +++ b/src/omnisharp/protocol.ts @@ -29,6 +29,7 @@ export module Requests { export const UpdateBuffer = '/updatebuffer'; export const Metadata = '/metadata'; export const ReAnalyze = '/reanalyze'; + export const QuickInfo = '/quickinfo'; } export namespace WireProtocol { @@ -473,6 +474,19 @@ export enum FileChangeType { DirectoryDelete = "DirectoryDelete" } +export interface QuickInfoRequest extends Request { +} + +export interface QuickInfoResponse { + Sections?: QuickInfoResponseSection[]; +} + +export interface QuickInfoResponseSection { + IsCSharpCode: boolean; + Text: string; +} + + export namespace V2 { export module Requests { @@ -491,7 +505,6 @@ export namespace V2 { export const BlockStructure = '/v2/blockstructure'; export const CodeStructure = '/v2/codestructure'; export const Highlight = '/v2/highlight'; - export const QuickInfo = '/v2/quickinfo'; } export interface SemanticHighlightSpan { @@ -712,20 +725,6 @@ export namespace V2 { Kind: string; } - export interface QuickInfoRequest extends Request { - } - - export interface QuickInfoResponse { - Description?: string; - Summary?: string; - RemainingSections?: QuickInfoResponseSection[]; - } - - export interface QuickInfoResponseSection { - IsCSharpCode: boolean; - Text: string; - } - export module SymbolKinds { // types export const Class = 'class'; diff --git a/src/omnisharp/utils.ts b/src/omnisharp/utils.ts index f4209ccba..c5d3bad52 100644 --- a/src/omnisharp/utils.ts +++ b/src/omnisharp/utils.ts @@ -169,8 +169,8 @@ export async function getSemanticHighlights(server: OmniSharpServer, request: pr return server.makeRequest(protocol.V2.Requests.Highlight, request); } -export async function getQuickInfo(server: OmniSharpServer, request: protocol.V2.QuickInfoRequest, token: CancellationToken) { - return server.makeRequest(protocol.V2.Requests.QuickInfo, request, token); +export async function getQuickInfo(server: OmniSharpServer, request: protocol.QuickInfoRequest, token: CancellationToken) { + return server.makeRequest(protocol.Requests.QuickInfo, request, token); } export async function isNetCoreProject(project: protocol.MSBuildProject) { From 8b93b9bba2ea93a35d88bdc7e9fa150ec7bfeb90 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Fri, 24 Jul 2020 18:27:49 -0700 Subject: [PATCH 3/5] React to server changes, update the integration tests. --- src/features/hoverProvider.ts | 15 +++------------ src/omnisharp/protocol.ts | 8 +------- .../hoverProvider.integration.test.ts | 6 +++--- 3 files changed, 7 insertions(+), 22 deletions(-) diff --git a/src/features/hoverProvider.ts b/src/features/hoverProvider.ts index 71bc150ad..2940f63cb 100644 --- a/src/features/hoverProvider.ts +++ b/src/features/hoverProvider.ts @@ -15,21 +15,12 @@ export default class OmniSharpHoverProvider extends AbstractSupport implements H let request = createRequest(document, position); try { const response = await serverUtils.getQuickInfo(this._server, request, token); - if (!response || !response.Sections) { + if (!response || !response.Markdown) { return undefined; } - let markdownString = new MarkdownString; - const language = "csharp"; - for (const section of response.Sections) { - if (section.IsCSharpCode) { - markdownString.appendCodeblock(section.Text, language); - } - else { - markdownString.appendText("\n"); - markdownString.appendMarkdown(section.Text); - } - } + const markdownString = new MarkdownString; + markdownString.appendMarkdown(response.Markdown); return new Hover(markdownString); } diff --git a/src/omnisharp/protocol.ts b/src/omnisharp/protocol.ts index 0ea77e1b0..d6cdd6e6d 100644 --- a/src/omnisharp/protocol.ts +++ b/src/omnisharp/protocol.ts @@ -478,15 +478,9 @@ export interface QuickInfoRequest extends Request { } export interface QuickInfoResponse { - Sections?: QuickInfoResponseSection[]; + Markdown?: string; } -export interface QuickInfoResponseSection { - IsCSharpCode: boolean; - Text: string; -} - - export namespace V2 { export module Requests { diff --git a/test/integrationTests/hoverProvider.integration.test.ts b/test/integrationTests/hoverProvider.integration.test.ts index dd6e342fc..0ca817972 100644 --- a/test/integrationTests/hoverProvider.integration.test.ts +++ b/test/integrationTests/hoverProvider.integration.test.ts @@ -40,8 +40,8 @@ suite(`Hover Provider: ${testAssetWorkspace.description}`, function () { await vscode.commands.executeCommand("vscode.open", fileUri); let c = (await vscode.commands.executeCommand("vscode.executeHoverProvider", fileUri, new vscode.Position(10, 29))); let answer: string = - `Checks if object is tagged with the tag.`; + "```csharp\nbool testissue.Compare(int gameObject, string tagName)\n```\n\nChecks if object is tagged with the tag.\n\nReturns:\n\n Returns true if object is tagged with tag."; - expect((<{ language: string; value: string }>c[0].contents[1]).value).to.equal(answer); + expect((<{ language: string; value: string }>c[0].contents[0]).value).to.equal(answer); }); -}); \ No newline at end of file +}); From 5f5ae8d5e44f6df648571aa7e31fd39529927800 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Sun, 26 Jul 2020 16:43:03 -0700 Subject: [PATCH 4/5] Remove unused import. --- test/integrationTests/hoverProvider.integration.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/integrationTests/hoverProvider.integration.test.ts b/test/integrationTests/hoverProvider.integration.test.ts index 0ca817972..6b3cc750a 100644 --- a/test/integrationTests/hoverProvider.integration.test.ts +++ b/test/integrationTests/hoverProvider.integration.test.ts @@ -10,6 +10,8 @@ import { should, expect } from 'chai'; import { activateCSharpExtension, isRazorWorkspace } from './integrationHelpers'; import testAssetWorkspace from './testAssets/testAssetWorkspace'; +export type Test = 'a' | 'b' | ['c', any]; + const chai = require('chai'); chai.use(require('chai-arrays')); chai.use(require('chai-fs')); From 5c8e7c70d5a490923af4587d1811a2b86bb71b98 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Mon, 10 Aug 2020 21:40:54 -0700 Subject: [PATCH 5/5] Update expected text after additional escaping was implemented on server side. --- test/integrationTests/hoverProvider.integration.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integrationTests/hoverProvider.integration.test.ts b/test/integrationTests/hoverProvider.integration.test.ts index 6b3cc750a..d6c0e8588 100644 --- a/test/integrationTests/hoverProvider.integration.test.ts +++ b/test/integrationTests/hoverProvider.integration.test.ts @@ -42,7 +42,7 @@ suite(`Hover Provider: ${testAssetWorkspace.description}`, function () { await vscode.commands.executeCommand("vscode.open", fileUri); let c = (await vscode.commands.executeCommand("vscode.executeHoverProvider", fileUri, new vscode.Position(10, 29))); let answer: string = - "```csharp\nbool testissue.Compare(int gameObject, string tagName)\n```\n\nChecks if object is tagged with the tag.\n\nReturns:\n\n Returns true if object is tagged with tag."; + "```csharp\nbool testissue.Compare(int gameObject, string tagName)\n```\n\nChecks if object is tagged with the tag\\.\n\nReturns:\n\n Returns true if object is tagged with tag\\."; expect((<{ language: string; value: string }>c[0].contents[0]).value).to.equal(answer); });