Merge pull request mathworks#17 from development/dlilley.UI_Improvements

Dlilley.UI improvements

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,15 +25,15 @@ class WorkspaceIndexer {
         }
 
         connection.workspace.onDidChangeWorkspaceFolders((params: WorkspaceFoldersChangeEvent) => {
-            this.handleWorkspaceFoldersAdded(params.added)
+            void this.handleWorkspaceFoldersAdded(params.added)
         })
     }
 
     /**
      * Attempts to index the files in the user's workspace.
      */
     async indexWorkspace (): Promise<void> {
-        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<void> {
+        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<boolean> {
+        const shouldIndexWorkspace = (await ConfigurationManager.getConfiguration()).indexWorkspace
+        return this.isWorkspaceIndexingSupported && shouldIndexWorkspace
     }
 }
 

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<Settings> {
+        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()

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()

src/lifecycle/MatlabCommunicationManager.ts

@@ -31,22 +31,21 @@ 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<MatlabProcessInfo> {
+    async connectToNewMatlab (launchCommand: string, launchArguments: string[], logDirectory: string, certificateDirectory?: string): Promise<MatlabProcessInfo> {
         const certDir = certificateDirectory ?? await fs.mkdtemp(path.join(os.tmpdir(), 'matlablsTmp-'))
         const port = await this._getAvailablePort()
         const certFile = path.join(certDir, 'cert.pem')
         const pkeyFile = path.join(certDir, 'pkey.p12')
         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
     }