Skip to content

Commit

Permalink
Reload the LS when relevant config changes
Browse files Browse the repository at this point in the history
  • Loading branch information
jribbink committed Jan 3, 2024
1 parent ada9db0 commit 2ace359
Show file tree
Hide file tree
Showing 10 changed files with 139 additions and 143 deletions.
2 changes: 1 addition & 1 deletion extension/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export async function activate (ctx: ExtensionContext): Promise<Extension | null

// Initialize the extension
Telemetry.withTelemetry(() => {
const settings = Settings.getWorkspaceSettings()
const settings = new Settings()
ext = Extension.initialize(settings, ctx)
})

Expand Down
4 changes: 2 additions & 2 deletions extension/src/server/flow-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class FlowConfig implements Disposable {
}

#resolveCustomConfigPath (): FlowConfigFile | null {
const customConfigPath = this.#settings.customConfigPath
const customConfigPath = this.#settings.getSettings().customConfigPath
if (customConfigPath === null || customConfigPath === '') {
return null
}
Expand Down Expand Up @@ -172,7 +172,7 @@ export class FlowConfig implements Disposable {

// Watch and reload flow configuration when changed.
#watchWorkspaceConfiguration (): Subscription {
return this.#settings.didChange$.subscribe(() => {
return this.#settings.watch$(config => config.customConfigPath).subscribe(() => {
void this.reloadConfigPath()
})
}
Expand Down
31 changes: 20 additions & 11 deletions extension/src/server/language-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ export class LanguageServerAPI {
settings: Settings

clientState$ = new BehaviorSubject<State>(State.Stopped)
#configPathSubscription: Subscription | null = null
#configModifiedSubscription: Subscription | null = null
#subscriptions: Subscription[] = []

#isActive = false

Expand All @@ -33,12 +32,12 @@ export class LanguageServerAPI {
this.#isActive = true
await this.startClient()
this.#subscribeToConfigChanges()
this.#subscribeToSettingsChanges()
}

async deactivate (): Promise<void> {
this.#isActive = false
this.#configPathSubscription?.unsubscribe()
this.#configModifiedSubscription?.unsubscribe()
this.#subscriptions.forEach((sub) => sub.unsubscribe())
await this.stopClient()
}

Expand All @@ -59,10 +58,10 @@ export class LanguageServerAPI {
// Set client state to starting
this.clientState$.next(State.Starting)

const accessCheckMode: string = this.settings.accessCheckMode
const accessCheckMode: string = this.settings.getSettings().accessCheckMode
const configPath: string | null = this.config.configPath

if (this.settings.flowCommand !== 'flow') {
if (this.settings.getSettings().flowCommand !== 'flow') {
try {
exec('killall dlv') // Required when running language server locally on mac
} catch (err) { void err }
Expand All @@ -73,7 +72,7 @@ export class LanguageServerAPI {
'cadence',
'Cadence',
{
command: this.settings.flowCommand,
command: this.settings.getSettings().flowCommand,
args: ['cadence', 'language-server', '--enable-flow-client=false'],
options: {
env
Expand Down Expand Up @@ -125,7 +124,7 @@ export class LanguageServerAPI {
#subscribeToConfigChanges (): void {
const tryReloadConfig = (): void => { void this.#sendRequest(RELOAD_CONFIGURATION).catch(() => {}) }

this.#configModifiedSubscription = this.config.fileModified$.subscribe(() => {
this.#subscriptions.push(this.config.fileModified$.subscribe(() => {
// Reload configuration
if (this.clientState$.getValue() === State.Running) {
tryReloadConfig()
Expand All @@ -135,12 +134,22 @@ export class LanguageServerAPI {
tryReloadConfig()
})
}
})
}))

this.#configPathSubscription = this.config.pathChanged$.subscribe(() => {
this.#subscriptions.push(this.config.pathChanged$.subscribe(() => {
// Restart client
void this.restart()
})
}))
}

#subscribeToSettingsChanges (): void {
const onChange = (): void => {
// Restart client
void this.restart()
}

this.#subscriptions.push(this.settings.watch$((config) => config.flowCommand).subscribe(onChange))
this.#subscriptions.push(this.settings.watch$((config) => config.accessCheckMode).subscribe(onChange))
}

async #sendRequest (cmd: string, args: any[] = []): Promise<any> {
Expand Down
130 changes: 49 additions & 81 deletions extension/src/settings/settings.ts
Original file line number Diff line number Diff line change
@@ -1,96 +1,64 @@
/* Workspace Settings */
import { homedir } from 'os'
import * as path from 'path'
import { Observable, Subject } from 'rxjs'
import { workspace, window } from 'vscode'
import { BehaviorSubject, Observable, distinctUntilChanged, map, skip } from 'rxjs'
import { workspace, Disposable } from 'vscode'
import { isEqual } from 'lodash'

export class Settings {
static CONFIG_FLOW_COMMAND = 'flowCommand'
static CONFIG_ACCESS_CHECK_MODE = 'accessCheckMode'
static CONFIG_CUSTOM_CONFIG_PATH = 'customConfigPath'

// Workspace settings singleton
static #instance: Settings | undefined

flowCommand!: string // The name of the Flow CLI executable.
accessCheckMode!: string
customConfigPath!: string // If empty then search the workspace for flow.json
maxTestConcurrency!: number

#didChange: Subject<void> = new Subject()
get didChange$ (): Observable<void> {
return this.#didChange.asObservable()
}

static getWorkspaceSettings (): Settings {
if (Settings.#instance === undefined) {
try {
Settings.#instance = new Settings()
} catch (err) {
window.showErrorMessage(`Failed to activate extension: ${String(err)}`)
.then(() => {}, () => {})
throw (Error('Could not retrieve workspace settings'))
}
}
return Settings.#instance
// Schema for the cadence configuration
export interface CadenceConfiguration {
flowCommand: string
accessCheckMode: string
customConfigPath: string
test: {
maxConcurrency: number
}
}

constructor (skipInitialization?: boolean) {
if (skipInitialization !== undefined && skipInitialization) {
return
}
export class Settings implements Disposable {
#configuration$: BehaviorSubject<CadenceConfiguration> = new BehaviorSubject<CadenceConfiguration>(this.#getConfiguration())
#disposables: Disposable[] = []

// Watch for workspace settings changes
workspace.onDidChangeConfiguration((e) => {
constructor () {
// Watch for configuration changes
this.#disposables.push(workspace.onDidChangeConfiguration((e) => {
if (e.affectsConfiguration('cadence')) {
this.#loadSettings()
this.#didChange.next()
this.#configuration$.next(this.#getConfiguration())
}
})

this.#loadSettings()
}))
}

#loadSettings (): void {
// Retrieve workspace settings
const cadenceConfig = workspace.getConfiguration('cadence')

const flowCommand: string | undefined = cadenceConfig.get(
Settings.CONFIG_FLOW_COMMAND
/**
* Returns an observable that emits whenever the configuration changes. If a selector is provided, the observable
* will only emit when the selected value changes.
*
* @param selector A function that selects a value from the configuration
* @returns An observable that emits whenever the configuration changes
* @template T The type of the selected value
* @example
* // Emit whenever the flow command changes
* settings.watch$(config => config.flowCommand)
*/
watch$<T = CadenceConfiguration> (selector: (config: CadenceConfiguration) => T = (config) => config as unknown as T): Observable<T> {
return this.#configuration$.asObservable().pipe(
skip(1),
map(selector),
distinctUntilChanged(isEqual)
)
if (flowCommand === undefined) {
throw new Error(`Missing ${Settings.CONFIG_FLOW_COMMAND} config`)
}
this.flowCommand = flowCommand
}

let accessCheckMode: string | undefined = cadenceConfig.get(
Settings.CONFIG_ACCESS_CHECK_MODE
)
if (accessCheckMode === undefined) {
accessCheckMode = 'strict'
}
this.accessCheckMode = accessCheckMode
/**
* Returns the current configuration
*
* @returns The current configuration
*/
getSettings (): CadenceConfiguration {
return this.#configuration$.value
}

let customConfigPath: string | undefined = cadenceConfig.get(
Settings.CONFIG_CUSTOM_CONFIG_PATH
)
if (customConfigPath === undefined) {
customConfigPath = ''
}
if (customConfigPath[0] === '~') {
customConfigPath = path.join(
homedir(),
customConfigPath.slice(1)
)
}
this.customConfigPath = customConfigPath
dispose (): void {
this.#configuration$.complete()
}

let maxTestConcurrency: number | undefined = cadenceConfig.get(
'maxTestConcurrency'
)
if (maxTestConcurrency === undefined) {
maxTestConcurrency = 5
}
this.maxTestConcurrency = maxTestConcurrency
#getConfiguration (): CadenceConfiguration {
return workspace.getConfiguration('cadence') as unknown as CadenceConfiguration
}
}
5 changes: 3 additions & 2 deletions extension/src/test-provider/test-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class TestRunner implements vscode.Disposable {
this.#settings = settings
this.#flowConfig = flowConfig
this.#testResolver = testResolver
this.#acquireLock = semaphore(settings.maxTestConcurrency)
this.#acquireLock = semaphore(settings.getSettings().test.maxConcurrency)

this.#disposibles.push(this.#controller.createRunProfile('Cadence Tests', vscode.TestRunProfileKind.Run, this.runTests.bind(this), true, CADENCE_TEST_TAG))
}
Expand Down Expand Up @@ -170,12 +170,13 @@ export class TestRunner implements vscode.Disposable {
if (cancellationToken?.isCancellationRequested === true) {
return {}
}

return await this.#acquireLock(async () => {
if (this.#flowConfig.configPath == null) {
throw new Error('Flow config path is null')
}
const args = ['test', `'${globPattern}'`, '--output=json', '-f', `'${this.#flowConfig.configPath}'`]
const { stdout, stderr } = await execDefault(this.#settings.flowCommand, args, { cwd: path.dirname(this.#flowConfig.configPath) }, cancellationToken)
const { stdout, stderr } = await execDefault(this.#settings.getSettings().flowCommand, args, { cwd: path.dirname(this.#flowConfig.configPath) }, cancellationToken)

if (stderr.length > 0) {
throw new Error(stderr)
Expand Down
45 changes: 12 additions & 33 deletions extension/test/integration/4 - flow-config.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@

import { Subject, firstValueFrom, of } from 'rxjs'
import { BehaviorSubject, firstValueFrom } from 'rxjs'
import { FlowConfig } from '../../src/server/flow-config'
import { Settings } from '../../src/settings/settings'
import { CadenceConfiguration, Settings } from '../../src/settings/settings'
import { before, afterEach } from 'mocha'
import * as assert from 'assert'
import * as path from 'path'
import * as fs from 'fs'
import { getMockSettings } from '../mock/mockSettings'

const workspacePath = path.resolve(__dirname, './fixtures/workspace')

Expand Down Expand Up @@ -43,53 +44,40 @@ suite('flow config tests', () => {
})

test('recognizes custom config path', async () => {
const mockSettings: Settings = {
customConfigPath: './foo/flow.json',
didChange$: of()
} as any
const mockSettings = getMockSettings({ customConfigPath: './foo/flow.json' })

await withConfig(mockSettings, (config) => {
assert.strictEqual(config.configPath, path.resolve(workspacePath, './foo/flow.json'))
})
})

test('recognizes config path from project root', async () => {
const mockSettings: Settings = {
customConfigPath: '',
didChange$: of()
} as any
const mockSettings = getMockSettings({ customConfigPath: '' })

await withConfig(mockSettings, (config) => {
assert.strictEqual(config.configPath, path.resolve(workspacePath, './flow.json'))
})
})

test('recognizes custom config change & emits pathChanged$', async () => {
const didChange$ = new Subject<void>()
const mockSettings = {
customConfigPath: './foo/flow.json',
didChange$: didChange$.asObservable()
} as any
const settings$ = new BehaviorSubject<Partial<CadenceConfiguration>>({ customConfigPath: './foo/flow.json' })
const mockSettings = getMockSettings(settings$)

await withConfig(mockSettings, async (config) => {
assert.strictEqual(config.configPath, path.resolve(workspacePath, './foo/flow.json'))

const pathChangedPromise = firstValueFrom(config.pathChanged$)

// update custom config path
mockSettings.customConfigPath = './bar/flow.json'
didChange$.next()
settings$.next({ customConfigPath: './bar/flow.json' })

await pathChangedPromise
assert.strictEqual(config.configPath, path.resolve(workspacePath, './bar/flow.json'))
})
})

test('ignores non-existent custom config path', async () => {
const mockSettings = {
customConfigPath: './missing/flow.json',
didChange$: of()
} as any
const mockSettings = getMockSettings({ customConfigPath: './missing/flow.json' })

await withConfig(mockSettings, (config) => {
assert.strictEqual(config.configPath, null)
Expand All @@ -100,10 +88,7 @@ suite('flow config tests', () => {
// temporarily delete config at root
deleteRootConfig()

const mockSettings: Settings = {
customConfigPath: '',
didChange$: of()
} as any
const mockSettings = getMockSettings({ customConfigPath: '' })

await withConfig(mockSettings, (config) => {
assert.strictEqual(config.configPath, null)
Expand All @@ -118,10 +103,7 @@ suite('flow config tests', () => {
setTimeout(resolve, 1000)
})

const mockSettings = {
customConfigPath: '',
didChange$: of()
} as any
const mockSettings = getMockSettings({ customConfigPath: '' })

await withConfig(mockSettings, async (config) => {
assert.strictEqual(config.configPath, null)
Expand All @@ -144,10 +126,7 @@ suite('flow config tests', () => {
setTimeout(resolve, 1000)
})

const mockSettings = {
customConfigPath: './missing/flow.json',
didChange$: of()
} as any
const mockSettings = getMockSettings({ customConfigPath: './missing/flow.json' })

await withConfig(mockSettings, async (config) => {
assert.strictEqual(config.configPath, null)
Expand Down
Loading

0 comments on commit 2ace359

Please sign in to comment.