Warn about unsupported shells (#1550)

* Warn about unsupported shells This PR introduces changes to do a best effort to determine if the shell is supported. Without exposing it to users yet.
stateful · Aug 28, 2024 · f40f661 · f40f661
1 parent ea2be63
commit f40f661
Show file tree

Hide file tree

Showing 11 changed files with 196 additions and 69 deletions.
diff --git a/package.json b/package.json
@@ -587,6 +587,12 @@
             "default": true,
             "markdownDescription": "If set to `true`, the extension will bring up the GRPC server."
           },
+          "runme.experiments.shellWarning": {
+            "type": "boolean",
+            "scope": "window",
+            "default": false,
+            "markdownDescription": "If set to `true`, the extension will display an error message if an unsupported shell is detected."
+          },
           "runme.checkout.projectDir": {
             "type": "string",
             "scope": "machine",
@@ -844,6 +850,12 @@
             "type": "string",
             "default": "http://localhost:8877/api",
             "description": "The base URL of the AI service."
+          },
+          "runme.app.docsUrl": {
+            "type": "string",
+            "scope": "window",
+            "default": "https://docs.runme.dev",
+            "markdownDescription": "Documentation Base URL"
           }
         }
       }

diff --git a/src/client/components/configuration/annotations.ts b/src/client/components/configuration/annotations.ts
@@ -2,7 +2,12 @@ import { LitElement, html } from 'lit'
 import { customElement, property } from 'lit/decorators.js'
 import { when } from 'lit/directives/when.js'
 
-import type { ClientMessage, CellAnnotations, CellAnnotationsErrorResult } from '../../../types'
+import type {
+  ClientMessage,
+  CellAnnotations,
+  CellAnnotationsErrorResult,
+  Settings,
+} from '../../../types'
 import { CellAnnotationsSchema, AnnotationSchema } from '../../../schema'
 import {
   ClientMessages,
@@ -90,6 +95,9 @@ export class Annotations extends LitElement {
   @property({ type: Array })
   categories: string[] = []
 
+  @property({ type: Object })
+  settings: Settings = {}
+
   #getTargetValue(e: Target) {
     switch (e.target.type) {
       case 'text':
@@ -228,7 +236,7 @@ export class Annotations extends LitElement {
   }
 
   renderDocsLink(id: string) {
-    const link = `https://docs.runme.dev/r/extension/${id}`
+    const link = `${this.settings?.docsUrl}/r/extension/${id}`
     return html`<vscode-link href="${link}">(docs ${ExternalLinkIcon})</vscode-link>`
   }
 
@@ -298,6 +306,7 @@ export class Annotations extends LitElement {
           identifier="${id}"
           @onChange=${this.onCategorySelectorChange}
           @onCreateNewCategory=${this.createNewCategoryClick}
+          @settings=${this.settings}
         ></category-selector>
       </div>
     </div>`

diff --git a/src/client/components/configuration/categorySelector.ts b/src/client/components/configuration/categorySelector.ts
@@ -5,6 +5,7 @@ import type { TextField } from '@vscode/webview-ui-toolkit'
 
 import { ExternalLinkIcon } from '../icons/external'
 import { CATEGORY_SEPARATOR } from '../../../constants'
+import { Settings } from '../../../types'
 
 export interface ISelectedCategory {
   name: string
@@ -302,6 +303,9 @@ export class CategorySelector extends LitElement {
   @property()
   identifier: string | undefined
 
+  @property({ type: Object })
+  settings: Settings = {}
+
   private dispatchComponentEvent(name: string, e: Event) {
     if (e.defaultPrevented) {
       e.preventDefault()
@@ -334,7 +338,8 @@ export class CategorySelector extends LitElement {
   }
 
   renderLink() {
-    return html`<vscode-link href="https://docs.runme.dev/configuration#run-all-cells-by-category"
+    return html`<vscode-link
+      href="${this.settings?.docsUrl}/configuration#run-all-cells-by-category"
       >(docs ${ExternalLinkIcon})</vscode-link
     >`
   }

diff --git a/src/client/index.ts b/src/client/index.ts
@@ -1,4 +1,4 @@
-import type { ActivationFunction, RendererContext } from 'vscode-notebook-renderer'
+import type { ActivationFunction } from 'vscode-notebook-renderer'
 
 import { OutputType, RENDERERS } from '../constants'
 import type { CellOutput } from '../types'
@@ -13,7 +13,7 @@ import './components'
 // rendering logic inside of the `render()` function.
 // ----------------------------------------------------------------------------
 
-export const activate: ActivationFunction = (context: RendererContext<void>) => {
+export const activate: ActivationFunction<void> = (context) => {
   setContext(context)
   return {
     renderOutputItem(outputItem, element) {
@@ -90,6 +90,7 @@ export const activate: ActivationFunction = (context: RendererContext<void>) =>
             'validationErrors',
             JSON.stringify(payload.output.validationErrors ?? []),
           )
+          annoElem.setAttribute('settings', JSON.stringify(payload.output.settings))
           element.appendChild(annoElem)
           break
         case OutputType.terminal:

diff --git a/src/extension/cell.ts b/src/extension/cell.ts
@@ -32,6 +32,7 @@ import {
 } from '../types'
 import { Mutex } from '../utils/sync'
 import {
+  getDocsUrl,
   getNotebookTerminalConfigurations,
   getSessionOutputs,
   isPlatformAuthEnabled,
@@ -169,6 +170,9 @@ export class NotebookCellOutputManager {
             annotations: getAnnotations(cell),
             validationErrors: validateAnnotations(cell),
             id: cell.metadata['runme.dev/id'],
+            settings: {
+              docsUrl: getDocsUrl(),
+            },
           },
         }
 

diff --git a/src/extension/executors/aws.ts b/src/extension/executors/aws.ts
@@ -14,7 +14,7 @@ import { resolveProgramOptionsScript } from './runner'
 import { IKernelExecutor } from '.'
 
 export const aws: IKernelExecutor = async (executor) => {
-  const { cellText, exec, runner, runnerEnv, doc, outputs, context } = executor
+  const { cellText, exec, runner, runnerEnv, doc, outputs, kernel } = executor
 
   const annotations = getAnnotations(exec.cell)
 
@@ -41,67 +41,19 @@ export const aws: IKernelExecutor = async (executor) => {
       cellId,
     })
 
-    // todo(sebastian): move down into kernel?
     switch (programOptions.exec?.type) {
       case 'script':
-        {
-          programOptions.exec.script = 'echo $AWS_PROFILE'
-        }
+        programOptions.exec.script = 'echo $AWS_PROFILE'
         break
     }
 
-    const program = await runner.createProgramSession(programOptions)
-    context.subscriptions.push(program)
+    let profile = ''
+    const result = await kernel.runProgram(programOptions)
 
-    let execRes: string | undefined
-    const onData = (data: string | Uint8Array) => {
-      if (execRes === undefined) {
-        execRes = ''
-      }
-      execRes += data.toString()
+    if (result) {
+      profile = result
     }
 
-    program.onDidWrite(onData)
-    program.onDidErr(onData)
-    program.run()
-
-    const success = await new Promise<boolean>((resolve, reject) => {
-      program.onDidClose(async (code) => {
-        if (code !== 0) {
-          return resolve(false)
-        }
-        return resolve(true)
-      })
-
-      program.onInternalErr((e) => {
-        reject(e)
-      })
-
-      const exitReason = program.hasExited()
-
-      // unexpected early return, likely an error
-      if (exitReason) {
-        switch (exitReason.type) {
-          case 'error':
-            {
-              reject(exitReason.error)
-            }
-            break
-
-          case 'exit':
-            {
-              resolve(exitReason.code === 0)
-            }
-            break
-
-          default: {
-            resolve(false)
-          }
-        }
-      }
-    })
-
-    const profile = success ? execRes?.trim() : 'default'
     credentials = fromIni({ profile })
 
     switch (awsResolver.view) {

diff --git a/src/extension/executors/resource.ts b/src/extension/executors/resource.ts
@@ -69,9 +69,8 @@ export const uri: IKernelRunner = async ({
 
   program.onDidWrite(onData)
   program.onDidErr(onData)
-  program.run()
 
-  const success = await new Promise<boolean>((resolve, reject) => {
+  const success = new Promise<boolean>((resolve, reject) => {
     program.onDidClose(async (code) => {
       if (code !== 0) {
         return resolve(false)
@@ -107,6 +106,9 @@ export const uri: IKernelRunner = async ({
     }
   })
 
-  const cellText = success ? execRes?.trim() : undefined
-  return runScript?.(cellText) || success
+  program.run()
+  const result = await success
+
+  const cellText = result ? execRes?.trim() : undefined
+  return runScript?.(cellText) || result
 }
diff --git a/src/extension/extension.ts b/src/extension/extension.ts
@@ -15,6 +15,7 @@ import Channel from 'tangle/webviews'
 
 import { NotebookUiEvent, Serializer, SyncSchema } from '../types'
 import {
+  getDocsUrlFor,
   getForceNewWindowConfig,
   getRunmeAppUrl,
   getSessionOutputs,
@@ -79,6 +80,8 @@ import { NotebookPanel as EnvStorePanel } from './panels/notebook'
 import { NotebookCellStatusBarProvider } from './provider/cellStatusBar/notebook'
 import { SessionOutputCellStatusBarProvider } from './provider/cellStatusBar/sessionOutput'
 import * as manager from './ai/manager'
+import getLogger from './logger'
+
 export class RunmeExtension {
   protected serializer?: SerializerBase
 
@@ -356,6 +359,40 @@ export class RunmeExtension {
     }
 
     await bootFile(context)
+
+    if (kernel.hasExperimentEnabled('shellWarning', false)) {
+      const showUnsupportedShellMessage = async () => {
+        const showMore = 'Show more'
+
+        const answer = await window.showErrorMessage('Unsupported shell', showMore)
+        if (answer === showMore) {
+          const url = getDocsUrlFor('/r/extension/supported-shells')
+          env.openExternal(Uri.parse(url))
+        }
+      }
+
+      const logger = getLogger('runme.beta.shellWarning')
+
+      kernel
+        .runProgram('echo $SHELL')
+        .then((output) => {
+          if (output === false) {
+            return
+          }
+
+          const supportedShells = ['bash', 'zsh']
+          const isSupported = supportedShells.some((sh) => output?.includes(sh))
+          logger.info(`Shell: ${output}`)
+
+          if (!isSupported) {
+            showUnsupportedShellMessage()
+          }
+        })
+        .catch((e) => {
+          logger.error(e)
+          showUnsupportedShellMessage()
+        })
+    }
   }
 
   protected handleMasking(kernel: Kernel, maskingIsOn: boolean): (e: NotebookUiEvent) => void {