Skip to content
This repository was archived by the owner on Apr 1, 2020. It is now read-only.

Retry symbol search whilst language server is still initialising #2634

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 48 additions & 25 deletions browser/src/Editor/NeovimEditor/Symbols.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@
* CodeAction.ts
*
*/

import * as _ from "lodash"
import { ErrorCodes } from "vscode-jsonrpc/lib/messages"
import * as types from "vscode-languageserver-types"

import * as Oni from "oni-api"

import { MenuManager } from "./../../Services/Menu"
import * as Log from "oni-core-logging"

import { LanguageManager } from "./../../Services/Language"
import { Menu, MenuManager } from "./../../Services/Menu"

import * as Helpers from "./../../Plugins/Api/LanguageClient/LanguageClientHelpers"

import { asObservable } from "./../../Utility"
import { asObservable, sleep } from "./../../Utility"

import { Definition } from "./Definition"

export class Symbols {
Expand Down Expand Up @@ -59,19 +61,9 @@ export class Symbols {
.debounceTime(25)
.do(() => menu.setLoading(true))
.concatMap(async (newText: string) => {
const buffer = this._editor.activeBuffer
const symbols: types.SymbolInformation[] = await this._languageManager.sendLanguageServerRequest(
buffer.language,
buffer.filePath,
"workspace/symbol",
{
textDocument: {
uri: Helpers.wrapPathInFileUri(buffer.filePath),
},
query: newText,
},
)
return symbols
return this._requestSymbols(this._editor.activeBuffer, "workspace/symbol", menu, {
query: newText,
})
})
.subscribe((newItems: types.SymbolInformation[]) => {
menu.setLoading(false)
Expand All @@ -94,15 +86,10 @@ export class Symbols {

const buffer = this._editor.activeBuffer

const result: types.SymbolInformation[] = await this._languageManager.sendLanguageServerRequest(
buffer.language,
buffer.filePath,
const result: types.SymbolInformation[] = await this._requestSymbols(
buffer,
"textDocument/documentSymbol",
{
textDocument: {
uri: Helpers.wrapPathInFileUri(buffer.filePath),
},
},
menu,
)

const options: Oni.Menu.MenuOption[] = result.map(item => this._symbolInfoToMenuItem(item))
Expand Down Expand Up @@ -175,4 +162,40 @@ export class Symbols {
return "question"
}
}

/**
* Send a request for symbols, retrying if the server is not ready, as long as the menu is open.
*/
private async _requestSymbols(
buffer: Oni.Buffer,
command: string,
menu: Menu,
options: any = {},
): Promise<types.SymbolInformation[]> {
while (menu.isOpen()) {
try {
return await this._languageManager.sendLanguageServerRequest(
buffer.language,
buffer.filePath,
command,
_.extend(
{
textDocument: {
uri: Helpers.wrapPathInFileUri(buffer.filePath),
},
},
options,
),
)
} catch (e) {
if (e.code === ErrorCodes.ServerNotInitialized) {
Log.warn("[Symbols] Language server not yet initialised, trying again...")
await sleep(1000)
} else {
throw e
}
}
}
return []
}
}
208 changes: 208 additions & 0 deletions browser/test/Editor/NeovimEditor/SymbolsTests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
import * as assert from "assert"
import * as path from "path"
import * as sinon from "sinon"

import { Event } from "oni-types"
import { ErrorCodes } from "vscode-jsonrpc/lib/messages"

import { Definition } from "../../../src/Editor/NeovimEditor/Definition"
import { Symbols } from "../../../src/Editor/NeovimEditor/Symbols"
import { wrapPathInFileUri } from "../../../src/Plugins/Api/LanguageClient/LanguageClientHelpers"
import { LanguageManager } from "../../../src/Services/Language"
import { Menu, MenuManager } from "../../../src/Services/Menu"

/* tslint:disable:no-string-literal */

const clock: any = global["clock"] // tslint:disable-line
const waitForPromiseResolution: any = global["waitForPromiseResolution"] // tslint:disable-line

describe("Symbols", () => {
let editor: any
let definition: any
let languageManager: any
let menuManager: any
let symbols: any

beforeEach(() => {
editor = sinon.stub()
editor.activeBuffer = "mock buffer"
definition = sinon.createStubInstance(Definition)
languageManager = sinon.createStubInstance(LanguageManager)
menuManager = sinon.createStubInstance(MenuManager)

symbols = new Symbols(editor, definition, languageManager, menuManager)
})

describe("open workspace/document menus", () => {
let menu: any
let onFilterTextChanged: Event<string>
let onItemSelected: Event<string>

beforeEach(() => {
menu = sinon.createStubInstance(Menu)
onFilterTextChanged = new Event<string>()
onItemSelected = new Event<string>()
sinon.stub(menu, "onItemSelected").get(() => onItemSelected)
sinon.stub(menu, "onFilterTextChanged").get(() => onFilterTextChanged)
menuManager.create.returns(menu)

symbols["_requestSymbols"] = sinon.stub().resolves(["first symbol", "second symbol"])
const _symbolInfoToMenuItem = sinon.stub()
_symbolInfoToMenuItem.onCall(0).returns("first transformed")
_symbolInfoToMenuItem.onCall(1).returns("second transformed")
symbols["_symbolInfoToMenuItem"] = _symbolInfoToMenuItem
})

describe("openWorkspaceSymbolsMenu", () => {
let getKey: any

beforeEach(() => {
getKey = sinon.stub()
symbols["_getDetailFromSymbol"] = sinon.stub().returns(getKey)
})

it("requests workspace symbols when filter text is changed", async () => {
// setup
symbols.openWorkspaceSymbolsMenu()
clock.tick(30)
sinon.assert.notCalled(symbols["_requestSymbols"])

// action
onFilterTextChanged.dispatch("mock query")

// confirm
clock.tick(24)
sinon.assert.notCalled(symbols["_requestSymbols"])
clock.tick(1)
sinon.assert.calledWithExactly(
symbols["_requestSymbols"],
"mock buffer",
"workspace/symbol",
menu,
{ query: "mock query" },
)
await waitForPromiseResolution()
assertCommon()
})
})

describe("openDocumentSymbolsMenu", () => {
it("requests document symbols when completion menu is opened", async () => {
// action
await symbols.openDocumentSymbolsMenu()

// confirm
sinon.assert.calledWithExactly(
symbols["_requestSymbols"],
"mock buffer",
"textDocument/documentSymbol",
menu,
)
assertCommon()
})
})

const assertCommon = () => {
assert.deepEqual(symbols["_symbolInfoToMenuItem"].args, [
["first symbol"],
["second symbol"],
])
sinon.assert.calledWithExactly(menu.setItems.lastCall, [
"first transformed",
"second transformed",
])
}
}) // End describe open menus

describe("_requestSymbols", () => {
let menu: any
let buffer: any

beforeEach(() => {
menu = sinon.createStubInstance(Menu)
menu.isOpen.returns(true)
buffer = sinon.stub()
buffer.language = "mocklang"
buffer.filePath = path.join("mock", "path")
})

it("throws on unknown errors", async () => {
// setup
const error = new Error()
languageManager.sendLanguageServerRequest.throws(error)

try {
// action
await symbols["_requestSymbols"](buffer, "mock command", menu)

// confirm
assert.fail("Expected exception to be thrown")
} catch (e) {
assert.strictEqual(e, error)
}
})

it("retries whilst server is initialising", async () => {
// setup
const error: any = new Error()
error.code = ErrorCodes.ServerNotInitialized
languageManager.sendLanguageServerRequest.onCall(0).throws(error)
languageManager.sendLanguageServerRequest.onCall(1).throws(error)
languageManager.sendLanguageServerRequest.onCall(2).returns("mock result")

// action
const request: Promise<any> = symbols["_requestSymbols"](buffer, "mock command", menu, {
mock: "option",
})

// confirm
sinon.assert.callCount(languageManager.sendLanguageServerRequest, 1)
clock.tick(999)
await waitForPromiseResolution()
sinon.assert.callCount(languageManager.sendLanguageServerRequest, 1)
clock.tick(1)
await waitForPromiseResolution()
sinon.assert.callCount(languageManager.sendLanguageServerRequest, 2)
clock.tick(1000)
await waitForPromiseResolution()
sinon.assert.callCount(languageManager.sendLanguageServerRequest, 3)
clock.tick(1000)
await waitForPromiseResolution()
sinon.assert.callCount(languageManager.sendLanguageServerRequest, 3)
clock.tick(1000)
await waitForPromiseResolution()
sinon.assert.alwaysCalledWith(
languageManager.sendLanguageServerRequest,
"mocklang",
buffer.filePath,
"mock command",
{ mock: "option", textDocument: { uri: wrapPathInFileUri(buffer.filePath) } },
)
assert.equal(await request, "mock result")
})

it("gives up retrying if menu is closed", async () => {
// setup
const error: any = new Error()
error.code = ErrorCodes.ServerNotInitialized
languageManager.sendLanguageServerRequest.throws(error)

// action
const request: Promise<any> = symbols["_requestSymbols"](buffer, "mock command", menu)

// confirm
sinon.assert.callCount(languageManager.sendLanguageServerRequest, 1)
clock.tick(1000)
await waitForPromiseResolution()
sinon.assert.callCount(languageManager.sendLanguageServerRequest, 2)
menu.isOpen.returns(false)
clock.tick(1000)
await waitForPromiseResolution()
sinon.assert.callCount(languageManager.sendLanguageServerRequest, 2)

const result = await request

assert.deepEqual(result, [])
})
})
})