From 139af18290a8d4f525bef48df5a7e2a392e38a3c Mon Sep 17 00:00:00 2001 From: Duncan Lilley Date: Tue, 13 Dec 2022 12:39:14 -0500 Subject: [PATCH 1/5] Implement notifications to alert user --- src/lifecycle/LifecycleNotificationHelper.ts | 33 +++++++ src/lifecycle/MatlabCommunicationManager.ts | 9 +- src/lifecycle/MatlabLifecycleManager.ts | 93 ++++++++++++------- src/notifications/NotificationService.ts | 37 ++++++++ .../formatting/FormatSupportProvider.ts | 2 + .../navigation/NavigationSupportProvider.ts | 2 + src/server.ts | 14 ++- 7 files changed, 145 insertions(+), 45 deletions(-) create mode 100644 src/lifecycle/LifecycleNotificationHelper.ts create mode 100644 src/notifications/NotificationService.ts diff --git a/src/lifecycle/LifecycleNotificationHelper.ts b/src/lifecycle/LifecycleNotificationHelper.ts new file mode 100644 index 0000000..aadd341 --- /dev/null +++ b/src/lifecycle/LifecycleNotificationHelper.ts @@ -0,0 +1,33 @@ +import NotificationService, { Notification } from '../notifications/NotificationService' + +export enum ConnectionState { + CONNECTING = 'connecting', + CONNECTED = 'connected', + DISCONNECTED = 'disconnected' +} + +class LifecycleNotificationHelper { + didMatlabLaunchFail = false + + /** + * Sends notification to the language client of a change in the MATLAB connection state. + * + * @param connectionStatus The connection state + */ + notifyConnectionStatusChange (connectionStatus: ConnectionState): void { + NotificationService.sendNotification(Notification.MatlabConnectionServerUpdate, { + connectionStatus + }) + } + + /** + * Sends notification to the language client to inform user that MATLAB is required for an action. + */ + notifyMatlabRequirement (): void { + // Indicate different messages if MATLAB failed to launch (i.e. could not be found) + const notification = this.didMatlabLaunchFail ? Notification.MatlabFeatureUnavailableNoMatlab : Notification.MatlabFeatureUnavailable + NotificationService.sendNotification(notification) + } +} + +export default new LifecycleNotificationHelper() diff --git a/src/lifecycle/MatlabCommunicationManager.ts b/src/lifecycle/MatlabCommunicationManager.ts index a246cc4..defbdac 100644 --- a/src/lifecycle/MatlabCommunicationManager.ts +++ b/src/lifecycle/MatlabCommunicationManager.ts @@ -31,12 +31,13 @@ class MatlabCommunicationManager { * Launches and connects to MATLAB. * * @param launchCommand The command with which MATLAB is launched + * @param launchArguments The arguments with which MATLAB is launched * @param logDirectory The directory in which MATLAB should log data * @param certificateDirectory The directory in which a certificate should be generated. * If no directory is provided, a temporary directory will be created. * @returns Information about the new MATLAB process and the connection to it. */ - async connectToNewMatlab (launchCommand: string, logDirectory: string, certificateDirectory?: string): Promise { + async connectToNewMatlab (launchCommand: string, launchArguments: string[], logDirectory: string, certificateDirectory?: string): Promise { const certDir = certificateDirectory ?? await fs.mkdtemp(path.join(os.tmpdir(), 'matlablsTmp-')) const port = await this._getAvailablePort() const certFile = path.join(certDir, 'cert.pem') @@ -44,9 +45,7 @@ class MatlabCommunicationManager { const apiKey = this._makeApiKey() // Spawn new instance of MATLAB - const matlabProcess = spawn(launchCommand, [], { - shell: true, - stdio: 'pipe', + const matlabProcess = spawn(launchCommand, launchArguments, { cwd: process.env.HOME, env: { ...process.env, @@ -133,7 +132,7 @@ export abstract class MatlabConnection { * Does not attempt to close MATLAB. */ close (): void { - this._client.disconnect() + this._client?.disconnect() this._lifecycleCallback = null } diff --git a/src/lifecycle/MatlabLifecycleManager.ts b/src/lifecycle/MatlabLifecycleManager.ts index 86812c8..d8447a9 100644 --- a/src/lifecycle/MatlabLifecycleManager.ts +++ b/src/lifecycle/MatlabLifecycleManager.ts @@ -8,6 +8,9 @@ import * as path from 'path' import MatlabCommunicationManager, { LifecycleEventType, MatlabConnection } from './MatlabCommunicationManager' import Logger from '../logging/Logger' import ArgumentManager, { Argument } from './ArgumentManager' +import { connection } from '../server' +import LifecycleNotificationHelper from './LifecycleNotificationHelper' +import NotificationService, { Notification } from '../notifications/NotificationService' export enum ConnectionTiming { Early = 'early', @@ -25,6 +28,10 @@ interface MatlabLifecycleEvent { matlabStatus: 'connected' | 'disconnected' } +export interface MatlabConnectionStatusParam { + connectionAction: 'connect' | 'disconnect' +} + type MatlabLifecycleCallback = (error: Error | null, evt: MatlabLifecycleEvent) => void /** @@ -120,6 +127,19 @@ class MatlabLifecycleManager { this._matlabLifecycleCallbacks.push(callback) } + /** + * Handles requests from the language client to either connect to or disconnect from MATLAB + * + * @param data Data about whether or not MATLAB should be connected or disconnected + */ + handleConnectionStatusChange (data: MatlabConnectionStatusParam): void { + if (data.connectionAction === 'connect') { + void this.connectToMatlab(connection) + } else if (data.connectionAction === 'disconnect') { + this.disconnectFromMatlab() + } + } + /** * Whether or not the language server should attempt to connect to an existing * MATLAB instance. @@ -235,14 +255,15 @@ class MatlabProcess { } this.isValid = false - this._notifyConnectionStatusChange(ConnectionState.DISCONNECTED) + LifecycleNotificationHelper.notifyConnectionStatusChange(ConnectionState.DISCONNECTED) } /** * Attempts to launch a new instance of MATLAB */ async launchMatlab (): Promise { - this._notifyConnectionStatusChange(ConnectionState.CONNECTING) + LifecycleNotificationHelper.didMatlabLaunchFail = false + LifecycleNotificationHelper.notifyConnectionStatusChange(ConnectionState.CONNECTING) return await new Promise(resolve => { const outFile = path.join(Logger.logDir, 'matlabls_conn.json') @@ -263,7 +284,7 @@ class MatlabProcess { this._matlabPid = info.matlabPid this._matlabConnection?.initialize().then(() => { fs.unwatchFile(outFile) - this._notifyConnectionStatusChange(ConnectionState.CONNECTED) + LifecycleNotificationHelper.notifyConnectionStatusChange(ConnectionState.CONNECTED) resolve() }).catch(reason => { Logger.error('Failed to connect to MATLAB') @@ -280,20 +301,20 @@ class MatlabProcess { * @param url The URL at which to find MATLAB */ async connectToMatlab (url: string): Promise { - this._notifyConnectionStatusChange(ConnectionState.CONNECTING) + LifecycleNotificationHelper.notifyConnectionStatusChange(ConnectionState.CONNECTING) this._matlabConnection = await MatlabCommunicationManager.connectToExistingMatlab(url) this._matlabConnection.setLifecycleListener(lifecycleEvent => { if (lifecycleEvent === LifecycleEventType.CONNECTED) { this._isReady = true - this._notifyConnectionStatusChange(ConnectionState.CONNECTED) + LifecycleNotificationHelper.notifyConnectionStatusChange(ConnectionState.CONNECTED) } else if (lifecycleEvent === LifecycleEventType.DISCONNECTED) { // Connection failed - retry after delay this._matlabConnection?.close() this._matlabConnection = null this._isReady = false - this._notifyConnectionStatusChange(ConnectionState.DISCONNECTED) + LifecycleNotificationHelper.notifyConnectionStatusChange(ConnectionState.DISCONNECTED) setTimeout(() => { void this.connectToMatlab(url) }, 1000) @@ -309,11 +330,11 @@ class MatlabProcess { * @param outFile The file in which MATLAB should output connection details */ private async _launchMatlabProcess (outFile: string): Promise { - const matlabLaunchCommand = this._getMatlabLaunchCommand(outFile) + const { command, args } = await this._getMatlabLaunchCommand(outFile) Logger.log('Launching MATLAB...') - const { matlabProcess, matlabConnection } = await MatlabCommunicationManager.connectToNewMatlab(matlabLaunchCommand, ArgumentManager.getArgument(Argument.MatlabCertificateDirectory) as string) + const { matlabProcess, matlabConnection } = await MatlabCommunicationManager.connectToNewMatlab(command, args, ArgumentManager.getArgument(Argument.MatlabCertificateDirectory) as string) this._matlabProcess = matlabProcess this._matlabConnection = matlabConnection @@ -333,13 +354,12 @@ class MatlabProcess { * This could include the user killing the process. */ this._matlabProcess.on('close', () => { - // TODO: Do we need to handle an exit code? // Close connection Logger.log('MATLAB process terminated') this._matlabConnection?.close() this.isValid = false - this._notifyConnectionStatusChange(ConnectionState.DISCONNECTED) + LifecycleNotificationHelper.notifyConnectionStatusChange(ConnectionState.DISCONNECTED) }) // Handles errors with the MATLAB process @@ -349,6 +369,9 @@ class MatlabProcess { if (error.stack != null) { Logger.error(`Error launching MATLAB: ${error.stack}`) } + + LifecycleNotificationHelper.didMatlabLaunchFail = true + NotificationService.sendNotification(Notification.MatlabLaunchFailed) }) this._matlabConnection.setLifecycleListener(lifecycleEvent => { @@ -357,7 +380,7 @@ class MatlabProcess { this._matlabConnection?.close() this.isValid = false - this._notifyConnectionStatusChange(ConnectionState.DISCONNECTED) + LifecycleNotificationHelper.notifyConnectionStatusChange(ConnectionState.DISCONNECTED) } }) } @@ -368,42 +391,48 @@ class MatlabProcess { * @param outFile The file in which MATLAB should output connection details * @returns The matlab launch command */ - private _getMatlabLaunchCommand (outFile: string): string { + private _getMatlabLaunchCommand (outFile: string): { command: string, args: string[] } { const matlabInstallPath = ArgumentManager.getArgument(Argument.MatlabInstallationPath) as string ?? '' - let launchCmd = 'matlab' + let command = 'matlab' if (matlabInstallPath !== '') { const matlabPath = path.normalize(path.join( matlabInstallPath, 'bin', 'matlab' )) - launchCmd = matlabPath.includes(' ') ? `"${matlabPath}"` : matlabPath + command = matlabPath.includes(' ') ? `"${matlabPath}"` : matlabPath } - let args = ArgumentManager.getArgument(Argument.MatlabLaunchCommandArguments) as string ?? '-nosplash' + const args: string[] = [] + + const argsFromSettings = ArgumentManager.getArgument(Argument.MatlabLaunchCommandArguments) as string ?? null + if (argsFromSettings != null) { + args.push(argsFromSettings) + } + + args.push('-nosplash') if (os.platform() === 'win32') { - args += ' -wait' + args.push('-wait') } - const startupFolderFlag = '-useStartupFolderPref' - const memManagerFlag = '-memmgr release' - const logFileFlag = `-logfile ${path.join(Logger.logDir, 'matlabls.log')}` - const runCmd = `-r "addpath(fullfile('${__dirname}', '..', 'matlab')); initmatlabls('${outFile}')"` - const desktopMode = os.platform() === 'win32' ? '-noDisplayDesktop' : '-nodesktop' // Workaround for Windows until better solution is implemented + args.push('-useStartupFolderPref') // Startup folder flag + args.push('-memmgr', 'release') // Memory manager + args.push('-logfile', path.join(Logger.logDir, 'matlabls.log')) // Log file + args.push('-r', `"addpath(fullfile('${__dirname}', '..', 'matlab')); initmatlabls('${outFile}')"`) // Startup command - return `${launchCmd} ${args} ${startupFolderFlag} ${memManagerFlag} ${logFileFlag} ${runCmd} ${desktopMode}` - } + // Desktop mode + if (os.platform() === 'win32') { + // Workaround for Windows until a better solution is implemented + args.push('-noDisplayDesktop') + } else { + args.push('-nodesktop') + } - /** - * Sends notification to the front-end of a change in the MATLAB connection state. - * - * @param connectionStatus The connection state - */ - private _notifyConnectionStatusChange (connectionStatus: ConnectionState): void { - void this._connection.sendNotification('matlab/connectionStatusChange', { - connectionStatus - }) + return { + command, + args + } } } diff --git a/src/notifications/NotificationService.ts b/src/notifications/NotificationService.ts new file mode 100644 index 0000000..9b5e772 --- /dev/null +++ b/src/notifications/NotificationService.ts @@ -0,0 +1,37 @@ +import { GenericNotificationHandler } from 'vscode-languageserver' +import { connection } from '../server' + +export enum Notification { + // Connection Status Updates + MatlabConnectionClientUpdate = 'matlab/connection/update/client', + MatlabConnectionServerUpdate = 'matlab/connection/update/server', + + // Errors + MatlabLaunchFailed = 'matlab/launchfailed', + MatlabFeatureUnavailable = 'feature/needsmatlab', + MatlabFeatureUnavailableNoMatlab = 'feature/needsmatlab/nomatlab' +} + +class NotificationService { + /** + * Sends a notification to the language client + * + * @param name The name of the notification + * @param params Any parameters to send with the notification + */ + sendNotification (name: string, params?: unknown): void { + void connection.sendNotification(name, params) + } + + /** + * Sets up a listener for notifications from the language client + * + * @param name The name of the notification + * @param callback The callback + */ + registerNotificationListener (name: string, callback: GenericNotificationHandler): void { + connection.onNotification(name, callback) + } +} + +export default new NotificationService() diff --git a/src/providers/formatting/FormatSupportProvider.ts b/src/providers/formatting/FormatSupportProvider.ts index b0cf04a..b8ec367 100644 --- a/src/providers/formatting/FormatSupportProvider.ts +++ b/src/providers/formatting/FormatSupportProvider.ts @@ -1,6 +1,7 @@ import { DocumentFormattingParams, HandlerResult, Position, Range, TextDocuments, TextEdit, _Connection } from 'vscode-languageserver' import { TextDocument } from 'vscode-languageserver-textdocument' +import LifecycleNotificationHelper from '../../lifecycle/LifecycleNotificationHelper' import MatlabLifecycleManager from '../../lifecycle/MatlabLifecycleManager' import * as TextDocumentUtils from '../../utils/TextDocumentUtils' @@ -47,6 +48,7 @@ class FormatSupportProvider { // If MATLAB is not available, no-op if (matlabConnection == null || !MatlabLifecycleManager.isMatlabReady()) { + LifecycleNotificationHelper.notifyMatlabRequirement() return [] } diff --git a/src/providers/navigation/NavigationSupportProvider.ts b/src/providers/navigation/NavigationSupportProvider.ts index 89d324e..5ff985e 100644 --- a/src/providers/navigation/NavigationSupportProvider.ts +++ b/src/providers/navigation/NavigationSupportProvider.ts @@ -9,6 +9,7 @@ import MatlabLifecycleManager from '../../lifecycle/MatlabLifecycleManager' import { getTextOnLine } from '../../utils/TextDocumentUtils' import PathResolver from './PathResolver' import { connection } from '../../server' +import LifecycleNotificationHelper from '../../lifecycle/LifecycleNotificationHelper' /** * Represents a code expression, either a single identifier or a dotted expression. @@ -76,6 +77,7 @@ class NavigationSupportProvider { async handleDefOrRefRequest (params: DefinitionParams | ReferenceParams, documentManager: TextDocuments, requestType: RequestType): Promise { const matlabConnection = await MatlabLifecycleManager.getOrCreateMatlabConnection(connection) if (matlabConnection == null) { + LifecycleNotificationHelper.notifyMatlabRequirement() return [] } diff --git a/src/server.ts b/src/server.ts index 86d7653..d4fff73 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3,8 +3,9 @@ import { ClientCapabilities, createConnection, InitializeParams, InitializeResul import DocumentIndexer from './indexing/DocumentIndexer' import WorkspaceIndexer from './indexing/WorkspaceIndexer' import ArgumentManager, { Argument } from './lifecycle/ArgumentManager' -import MatlabLifecycleManager, { ConnectionTiming } from './lifecycle/MatlabLifecycleManager' +import MatlabLifecycleManager, { ConnectionTiming, MatlabConnectionStatusParam } from './lifecycle/MatlabLifecycleManager' import Logger from './logging/Logger' +import NotificationService, { Notification } from './notifications/NotificationService' import CompletionProvider from './providers/completion/CompletionSupportProvider' import FormatSupportProvider from './providers/formatting/FormatSupportProvider' import LintingSupportProvider from './providers/linting/LintingSupportProvider' @@ -95,13 +96,10 @@ connection.onShutdown(() => { }) // Set up connection notification listeners -connection.onNotification('matlab/updateConnection', (data: { connectionAction: 'connect' | 'disconnect' }) => { - if (data.connectionAction === 'connect') { - void MatlabLifecycleManager.connectToMatlab(connection) - } else if (data.connectionAction === 'disconnect') { - MatlabLifecycleManager.disconnectFromMatlab() - } -}) +NotificationService.registerNotificationListener( + Notification.MatlabConnectionClientUpdate, + data => MatlabLifecycleManager.handleConnectionStatusChange(data as MatlabConnectionStatusParam) +) // Handles files opened documentManager.onDidOpen(params => { From 2c5a0107e15a5251a9dbcdcef3c1c3a5dfd6ae40 Mon Sep 17 00:00:00 2001 From: Duncan Lilley Date: Tue, 13 Dec 2022 12:53:37 -0500 Subject: [PATCH 2/5] Sync configuration with language client --- src/indexing/WorkspaceIndexer.ts | 18 ++- src/lifecycle/ArgumentManager.ts | 51 ------- src/lifecycle/ConfigurationManager.ts | 125 ++++++++++++++++++ src/lifecycle/MatlabLifecycleManager.ts | 17 +-- .../linting/LintingSupportProvider.ts | 4 +- src/server.ts | 18 +-- src/utils/CliUtils.ts | 2 +- 7 files changed, 155 insertions(+), 80 deletions(-) delete mode 100644 src/lifecycle/ArgumentManager.ts create mode 100644 src/lifecycle/ConfigurationManager.ts diff --git a/src/indexing/WorkspaceIndexer.ts b/src/indexing/WorkspaceIndexer.ts index 1566a87..7c64c19 100644 --- a/src/indexing/WorkspaceIndexer.ts +++ b/src/indexing/WorkspaceIndexer.ts @@ -1,5 +1,5 @@ import { ClientCapabilities, WorkspaceFolder, WorkspaceFoldersChangeEvent } from 'vscode-languageserver' -import ArgumentManager, { Argument } from '../lifecycle/ArgumentManager' +import ConfigurationManager from '../lifecycle/ConfigurationManager' import { connection } from '../server' import Indexer from './Indexer' @@ -8,9 +8,6 @@ import Indexer from './Indexer' * functions, and variables. */ class WorkspaceIndexer { - private readonly REQUEST_CHANNEL = '/matlabls/indexWorkspace/request' - private readonly RESPONSE_CHANNEL = '/matlabls/indexWorkspace/response/' // Needs to be appended with requestId - private isWorkspaceIndexingSupported = false /** @@ -28,7 +25,7 @@ class WorkspaceIndexer { } connection.workspace.onDidChangeWorkspaceFolders((params: WorkspaceFoldersChangeEvent) => { - this.handleWorkspaceFoldersAdded(params.added) + void this.handleWorkspaceFoldersAdded(params.added) }) } @@ -36,7 +33,7 @@ class WorkspaceIndexer { * Attempts to index the files in the user's workspace. */ async indexWorkspace (): Promise { - if (!this.shouldIndexWorkspace()) { + if (!(await this.shouldIndexWorkspace())) { return } @@ -54,8 +51,8 @@ class WorkspaceIndexer { * * @param folders The list of folders added to the workspace */ - private handleWorkspaceFoldersAdded (folders: WorkspaceFolder[]): void { - if (!this.shouldIndexWorkspace()) { + private async handleWorkspaceFoldersAdded (folders: WorkspaceFolder[]): Promise { + if (!(await this.shouldIndexWorkspace())) { return } @@ -69,8 +66,9 @@ class WorkspaceIndexer { * * @returns True if workspace indexing should occurr, false otherwise. */ - private shouldIndexWorkspace (): boolean { - return this.isWorkspaceIndexingSupported && ArgumentManager.getArgument(Argument.ShouldIndexWorkspace) as boolean + private async shouldIndexWorkspace (): Promise { + const shouldIndexWorkspace = (await ConfigurationManager.getConfiguration()).indexWorkspace + return this.isWorkspaceIndexingSupported && shouldIndexWorkspace } } diff --git a/src/lifecycle/ArgumentManager.ts b/src/lifecycle/ArgumentManager.ts deleted file mode 100644 index 5612bc8..0000000 --- a/src/lifecycle/ArgumentManager.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { CliArgs } from '../utils/CliUtils' - -export enum Argument { - // Basic arguments - MatlabLaunchCommandArguments = 'matlabLaunchCommandArgs', - MatlabCertificateDirectory = 'matlabCertDir', - MatlabInstallationPath = 'matlabInstallPath', - MatlabConnectionTiming = 'matlabConnectionTiming', - - ShouldIndexWorkspace = 'indexWorkspace', - - // Advanced arguments - MatlabUrl = 'matlabUrl' -} - -class ArgumentManager { - private readonly _argMap = new Map() - - /** - * Initialize the map with the provided arguments - * - * @param cliArgs The parsed CLI arguments - */ - initializeFromCliArguments (cliArgs: CliArgs): void { - Object.values(Argument).forEach(arg => { - this.setArgument(arg, cliArgs[arg]) - }) - } - - /** - * Sets an argument to a new value - * - * @param argument The argument being set - * @param value The argument's new value - */ - setArgument (argument: Argument, value: unknown): void { - this._argMap.set(argument, value) - } - - /** - * Gets the value of the given argument - * - * @param argument The argument - * @returns The argument's value - */ - getArgument (argument: Argument): unknown { - return this._argMap.get(argument) - } -} - -export default new ArgumentManager() diff --git a/src/lifecycle/ConfigurationManager.ts b/src/lifecycle/ConfigurationManager.ts new file mode 100644 index 0000000..689e9ea --- /dev/null +++ b/src/lifecycle/ConfigurationManager.ts @@ -0,0 +1,125 @@ +import { ClientCapabilities, DidChangeConfigurationNotification, DidChangeConfigurationParams } from 'vscode-languageserver' +import { connection } from '../server' +import { getCliArgs } from '../utils/CliUtils' + +export enum Argument { + // Basic arguments + MatlabLaunchCommandArguments = 'matlabLaunchCommandArgs', + MatlabCertificateDirectory = 'matlabCertDir', + MatlabInstallationPath = 'matlabInstallPath', + MatlabConnectionTiming = 'matlabConnectionTiming', + + ShouldIndexWorkspace = 'indexWorkspace', + + // Advanced arguments + MatlabUrl = 'matlabUrl' +} + +export enum ConnectionTiming { + Early = 'early', + Late = 'late', + Never = 'never' +} + +interface CliArguments { + [Argument.MatlabLaunchCommandArguments]: string + [Argument.MatlabCertificateDirectory]: string + [Argument.MatlabUrl]: string +} + +interface Settings { + installPath: string + matlabConnectionTiming: ConnectionTiming + indexWorkspace: boolean +} + +class ConfigurationManager { + private configuration: Settings | null = null + private readonly defaultConfiguration: Settings + private globalSettings: Settings + + // Holds additional command line arguments that are not part of the configuration + private readonly additionalArguments: CliArguments + + private hasConfigurationCapability = false + + constructor () { + const cliArgs = getCliArgs() + + this.defaultConfiguration = { + installPath: '', + matlabConnectionTiming: ConnectionTiming.Early, + indexWorkspace: false + } + + this.globalSettings = { + installPath: cliArgs[Argument.MatlabInstallationPath] ?? this.defaultConfiguration.installPath, + matlabConnectionTiming: cliArgs[Argument.MatlabConnectionTiming] as ConnectionTiming ?? this.defaultConfiguration.matlabConnectionTiming, + indexWorkspace: cliArgs[Argument.ShouldIndexWorkspace] ?? this.defaultConfiguration.indexWorkspace + } + + this.additionalArguments = { + [Argument.MatlabLaunchCommandArguments]: cliArgs[Argument.MatlabLaunchCommandArguments] ?? '', + [Argument.MatlabCertificateDirectory]: cliArgs[Argument.MatlabCertificateDirectory] ?? '', + [Argument.MatlabUrl]: cliArgs[Argument.MatlabUrl] ?? '' + } + } + + /** + * Sets up the configuration manager + * + * @param capabilities The client capabilities + */ + setup (capabilities: ClientCapabilities): void { + this.hasConfigurationCapability = capabilities.workspace?.configuration != null + + if (this.hasConfigurationCapability) { + // Register for configuration changes + void connection.client.register(DidChangeConfigurationNotification.type) + } + + connection.onDidChangeConfiguration(params => this.handleConfigurationChanged(params)) + } + + /** + * Gets the configuration for the langauge server + * + * @returns The current configuration + */ + async getConfiguration (): Promise { + if (this.hasConfigurationCapability) { + if (this.configuration == null) { + this.configuration = await connection.workspace.getConfiguration('matlab') as Settings + } + + return this.configuration + } + + return this.globalSettings + } + + /** + * Gets the value of the given argument + * + * @param argument The argument + * @returns The argument's value + */ + getArgument (argument: Argument.MatlabLaunchCommandArguments | Argument.MatlabCertificateDirectory | Argument.MatlabUrl): string { + return this.additionalArguments[argument] + } + + /** + * Handles a change in the configuration + * @param params The configuration changed params + */ + private handleConfigurationChanged (params: DidChangeConfigurationParams): void { + if (this.hasConfigurationCapability) { + // Clear cached configuration + this.configuration = null + } else { + this.globalSettings = params.settings?.matlab ?? this.defaultConfiguration + } + } +} + +export default new ConfigurationManager() diff --git a/src/lifecycle/MatlabLifecycleManager.ts b/src/lifecycle/MatlabLifecycleManager.ts index d8447a9..a5511c0 100644 --- a/src/lifecycle/MatlabLifecycleManager.ts +++ b/src/lifecycle/MatlabLifecycleManager.ts @@ -7,7 +7,7 @@ import * as path from 'path' import MatlabCommunicationManager, { LifecycleEventType, MatlabConnection } from './MatlabCommunicationManager' import Logger from '../logging/Logger' -import ArgumentManager, { Argument } from './ArgumentManager' +import ConfigurationManager, { Argument } from './ConfigurationManager' import { connection } from '../server' import LifecycleNotificationHelper from './LifecycleNotificationHelper' import NotificationService, { Notification } from '../notifications/NotificationService' @@ -108,7 +108,8 @@ class MatlabLifecycleManager { } // No active connection - should create a connection if desired - if (ArgumentManager.getArgument(Argument.MatlabConnectionTiming) !== ConnectionTiming.Never) { + const connectionTiming = (await ConfigurationManager.getConfiguration()).matlabConnectionTiming + if (connectionTiming !== ConnectionTiming.Never) { const matlabProcess = await this.connectToMatlab(connection) return matlabProcess.getConnection() } @@ -149,7 +150,7 @@ class MatlabLifecycleManager { */ private _shouldConnectToExistingMatlab (): boolean { // Assume we should connect to existing MATLAB if the matlabUrl startup flag has been provided - return Boolean(ArgumentManager.getArgument(Argument.MatlabUrl)) + return Boolean(ConfigurationManager.getArgument(Argument.MatlabUrl)) } /** @@ -159,7 +160,7 @@ class MatlabLifecycleManager { * @returns The connected MATLAB process */ private async _connectToExistingMatlab (connection: _Connection): Promise { - const url = ArgumentManager.getArgument(Argument.MatlabUrl) as string + const url = ConfigurationManager.getArgument(Argument.MatlabUrl) if (this._matlabProcess == null || !this._matlabProcess.isValid) { this._matlabProcess = new MatlabProcess(connection) @@ -334,7 +335,7 @@ class MatlabProcess { Logger.log('Launching MATLAB...') - const { matlabProcess, matlabConnection } = await MatlabCommunicationManager.connectToNewMatlab(command, args, ArgumentManager.getArgument(Argument.MatlabCertificateDirectory) as string) + const { matlabProcess, matlabConnection } = await MatlabCommunicationManager.connectToNewMatlab(command, args, ConfigurationManager.getArgument(Argument.MatlabCertificateDirectory)) this._matlabProcess = matlabProcess this._matlabConnection = matlabConnection @@ -391,8 +392,8 @@ class MatlabProcess { * @param outFile The file in which MATLAB should output connection details * @returns The matlab launch command */ - private _getMatlabLaunchCommand (outFile: string): { command: string, args: string[] } { - const matlabInstallPath = ArgumentManager.getArgument(Argument.MatlabInstallationPath) as string ?? '' + private async _getMatlabLaunchCommand (outFile: string): Promise<{ command: string, args: string[] }> { + const matlabInstallPath = (await ConfigurationManager.getConfiguration()).installPath let command = 'matlab' if (matlabInstallPath !== '') { const matlabPath = path.normalize(path.join( @@ -405,7 +406,7 @@ class MatlabProcess { const args: string[] = [] - const argsFromSettings = ArgumentManager.getArgument(Argument.MatlabLaunchCommandArguments) as string ?? null + const argsFromSettings = ConfigurationManager.getArgument(Argument.MatlabLaunchCommandArguments) ?? null if (argsFromSettings != null) { args.push(argsFromSettings) } diff --git a/src/providers/linting/LintingSupportProvider.ts b/src/providers/linting/LintingSupportProvider.ts index f2b278f..88845c5 100644 --- a/src/providers/linting/LintingSupportProvider.ts +++ b/src/providers/linting/LintingSupportProvider.ts @@ -2,7 +2,7 @@ import { execFile, ExecFileException } from 'child_process' import { CodeAction, CodeActionKind, CodeActionParams, Command, Diagnostic, DiagnosticSeverity, Position, Range, TextDocumentEdit, TextEdit, VersionedTextDocumentIdentifier, WorkspaceEdit, _Connection } from 'vscode-languageserver' import { TextDocument } from 'vscode-languageserver-textdocument' import { URI } from 'vscode-uri' -import ArgumentManager, { Argument } from '../../lifecycle/ArgumentManager' +import ConfigurationManager from '../../lifecycle/ConfigurationManager' import { MatlabConnection } from '../../lifecycle/MatlabCommunicationManager' import MatlabLifecycleManager from '../../lifecycle/MatlabLifecycleManager' import Logger from '../../logging/Logger' @@ -426,7 +426,7 @@ class LintingSupportProvider { return null } - const matlabInstallPath = ArgumentManager.getArgument(Argument.MatlabInstallationPath) as string + const matlabInstallPath = (await ConfigurationManager.getConfiguration()).installPath let binPath = '' if (matlabInstallPath !== '') { diff --git a/src/server.ts b/src/server.ts index d4fff73..7190a51 100644 --- a/src/server.ts +++ b/src/server.ts @@ -2,7 +2,7 @@ import { TextDocument } from 'vscode-languageserver-textdocument' import { ClientCapabilities, createConnection, InitializeParams, InitializeResult, ProposedFeatures, TextDocuments } from 'vscode-languageserver/node' import DocumentIndexer from './indexing/DocumentIndexer' import WorkspaceIndexer from './indexing/WorkspaceIndexer' -import ArgumentManager, { Argument } from './lifecycle/ArgumentManager' +import ConfigurationManager, { Argument } from './lifecycle/ConfigurationManager' import MatlabLifecycleManager, { ConnectionTiming, MatlabConnectionStatusParam } from './lifecycle/MatlabLifecycleManager' import Logger from './logging/Logger' import NotificationService, { Notification } from './notifications/NotificationService' @@ -11,7 +11,6 @@ import FormatSupportProvider from './providers/formatting/FormatSupportProvider' import LintingSupportProvider from './providers/linting/LintingSupportProvider' import ExecuteCommandProvider, { MatlabLSCommands } from './providers/lspCommands/ExecuteCommandProvider' import NavigationSupportProvider, { RequestType } from './providers/navigation/NavigationSupportProvider' -import { getCliArgs } from './utils/CliUtils' // Create a connection for the server export const connection = createConnection(ProposedFeatures.all) @@ -22,10 +21,6 @@ Logger.initialize(connection.console) // Create basic text document manager const documentManager: TextDocuments = new TextDocuments(TextDocument) -// Get CLI arguments -const cliArgs = getCliArgs() -ArgumentManager.initializeFromCliArguments(cliArgs) - MatlabLifecycleManager.addMatlabLifecycleListener((error, lifecycleEvent) => { if (error != null) { Logger.error(`MATLAB Lifecycle Error: ${error.message}\n${error.stack ?? ''}`) @@ -81,13 +76,20 @@ connection.onInitialize((params: InitializeParams) => { // Handles the initialized notification connection.onInitialized(() => { + ConfigurationManager.setup(capabilities) + WorkspaceIndexer.setupCallbacks(capabilities) + void startMatlabIfEarlyLaunch() +}) + +async function startMatlabIfEarlyLaunch (): Promise { // Launch MATLAB if it should be launched early - if (ArgumentManager.getArgument(Argument.MatlabConnectionTiming) === ConnectionTiming.Early) { + const connectionTiming = (await ConfigurationManager.getConfiguration()).matlabConnectionTiming + if (connectionTiming === ConnectionTiming.Early) { void MatlabLifecycleManager.connectToMatlab(connection) } -}) +} // Handles a shutdown request connection.onShutdown(() => { diff --git a/src/utils/CliUtils.ts b/src/utils/CliUtils.ts index 3c6a0ff..2dffefd 100644 --- a/src/utils/CliUtils.ts +++ b/src/utils/CliUtils.ts @@ -1,5 +1,5 @@ import * as yargs from 'yargs' -import { Argument } from '../lifecycle/ArgumentManager' +import { Argument } from '../lifecycle/ConfigurationManager' export interface CliArgs { [Argument.MatlabLaunchCommandArguments]?: string From d74a2aa01253942dc585d7f60a7a32e700a74e5d Mon Sep 17 00:00:00 2001 From: Duncan Lilley Date: Tue, 13 Dec 2022 14:59:04 -0500 Subject: [PATCH 3/5] Removed unused import --- src/server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.ts b/src/server.ts index 7190a51..942f306 100644 --- a/src/server.ts +++ b/src/server.ts @@ -2,7 +2,7 @@ import { TextDocument } from 'vscode-languageserver-textdocument' import { ClientCapabilities, createConnection, InitializeParams, InitializeResult, ProposedFeatures, TextDocuments } from 'vscode-languageserver/node' import DocumentIndexer from './indexing/DocumentIndexer' import WorkspaceIndexer from './indexing/WorkspaceIndexer' -import ConfigurationManager, { Argument } from './lifecycle/ConfigurationManager' +import ConfigurationManager from './lifecycle/ConfigurationManager' import MatlabLifecycleManager, { ConnectionTiming, MatlabConnectionStatusParam } from './lifecycle/MatlabLifecycleManager' import Logger from './logging/Logger' import NotificationService, { Notification } from './notifications/NotificationService' From 0c6a689453586e7cdcf862afac519cdeba90d0bf Mon Sep 17 00:00:00 2001 From: Duncan Lilley Date: Tue, 13 Dec 2022 14:59:59 -0500 Subject: [PATCH 4/5] Converted unnecessary 'else if' to simply 'else' --- src/lifecycle/MatlabLifecycleManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lifecycle/MatlabLifecycleManager.ts b/src/lifecycle/MatlabLifecycleManager.ts index a5511c0..bbdfba3 100644 --- a/src/lifecycle/MatlabLifecycleManager.ts +++ b/src/lifecycle/MatlabLifecycleManager.ts @@ -136,7 +136,7 @@ class MatlabLifecycleManager { handleConnectionStatusChange (data: MatlabConnectionStatusParam): void { if (data.connectionAction === 'connect') { void this.connectToMatlab(connection) - } else if (data.connectionAction === 'disconnect') { + } else { this.disconnectFromMatlab() } } From 7c672780b8bab64a474d36037b04217312bce755 Mon Sep 17 00:00:00 2001 From: Duncan Lilley Date: Tue, 13 Dec 2022 16:11:53 -0500 Subject: [PATCH 5/5] Restructure building matlab launch args --- src/lifecycle/MatlabLifecycleManager.ts | 32 ++++++++++--------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/lifecycle/MatlabLifecycleManager.ts b/src/lifecycle/MatlabLifecycleManager.ts index bbdfba3..75737b3 100644 --- a/src/lifecycle/MatlabLifecycleManager.ts +++ b/src/lifecycle/MatlabLifecycleManager.ts @@ -404,32 +404,26 @@ class MatlabProcess { command = matlabPath.includes(' ') ? `"${matlabPath}"` : matlabPath } - const args: string[] = [] - - const argsFromSettings = ConfigurationManager.getArgument(Argument.MatlabLaunchCommandArguments) ?? null - if (argsFromSettings != null) { - args.push(argsFromSettings) - } - - args.push('-nosplash') + const args = [ + '-nosplash', + '-useStartupFolderPref', // Startup folder flag + '-memmgr', 'release', // Memory manager + '-logFile', path.join(Logger.logDir, 'matlabls.log'), // Log file + '-r', `"addpath(fullfile('${__dirname}', '..', 'matlab')); initmatlabls('${outFile}')"` // Startup command + ] if (os.platform() === 'win32') { args.push('-wait') - } - - args.push('-useStartupFolderPref') // Startup folder flag - args.push('-memmgr', 'release') // Memory manager - args.push('-logfile', path.join(Logger.logDir, 'matlabls.log')) // Log file - args.push('-r', `"addpath(fullfile('${__dirname}', '..', 'matlab')); initmatlabls('${outFile}')"`) // Startup command - - // Desktop mode - if (os.platform() === 'win32') { - // Workaround for Windows until a better solution is implemented - args.push('-noDisplayDesktop') + args.push('-noDisplayDesktop') // Workaround for '-nodesktop' on Windows until a better solution is implemented } else { args.push('-nodesktop') } + const argsFromSettings = ConfigurationManager.getArgument(Argument.MatlabLaunchCommandArguments) ?? null + if (argsFromSettings != null) { + args.push(argsFromSettings) + } + return { command, args