Refactor Output Actions: Auto-Save, Save, Preview & Gist (#1868)

* Introduced a new `save-button` component to simplify and separate save functionality from `action-button`.

* WIP - Detaching Preview outputs from AutoSave

* Repository sync

* Sync repository

* Updating tests

* Repository sync

* Achieve the global threshold coverage for branches.

- Added shouldSkipRefreshTerminal method to handle terminal refresh conditions.
- Updated refreshTerminal method to use shouldSkipRefreshTerminal.
- Added unit tests for shouldSkipRefreshTerminal and refreshTerminal methods.

* extract notebook outputs preview into separate command

* Update shouldWriteOutputs logic to handle edge cases

package.json

@@ -365,7 +365,7 @@
         "category": "Runme",
         "title": "Click to open session outputs",
         "icon": "$(output-view-icon)",
-        "shortTitle": "Session Outputs"
+        "shortTitle": "Preview Outputs"
       },
       {
         "command": "runme.resetRunnerSession",
@@ -585,7 +585,7 @@
       "notebook/toolbar": [
         {
           "command": "runme.notebookSessionOutputs",
-          "when": "notebookType == runme && notebookMode != sessionOutputs && notebookHasRunmeOutputs",
+          "when": "notebookType == runme && notebookMode != sessionOutputs",
           "group": "navigation"
         },
         {

src/client/components/terminal/actionButton.ts

@@ -20,10 +20,10 @@ export class ActionButton extends LitElement {
   disabled: boolean = false
 
   @property({ type: Boolean, reflect: true })
-  shareIcon: boolean = false
+  shareIcon: boolean | undefined
 
   @property({ type: Boolean, reflect: true })
-  saveIcon: boolean = false
+  saveIcon: boolean | undefined
 
   /* eslint-disable */
   static styles = css`

src/client/components/terminal/gistCell.ts

@@ -7,7 +7,7 @@ import { GistIcon } from '../icons/gistIcon'
 @customElement('gist-cell')
 export class GistCell extends LitElement {
   @property({ type: String })
-  text: string = 'Preview Gist'
+  text: string = 'Preview & Gist'
 
   @property({ type: Boolean, reflect: true })
   disabled: boolean = false

src/client/components/terminal/index.ts

@@ -27,6 +27,8 @@ import '../copyButton'
 import './actionButton'
 import './gistCell'
 import './open'
+import './saveButton'
+import './shareButton'
 import {
   CreateCellExecutionMutation,
   CreateEscalationMutation,
@@ -389,9 +391,6 @@ export class TerminalView extends LitElement {
   @property({ type: Boolean })
   isAutoSaveEnabled: boolean = false
 
-  @property({ type: Boolean })
-  isSessionOutputsEnabled: boolean = false
-
   @property({ type: Boolean })
   isPlatformAuthEnabled: boolean = false
 
@@ -594,12 +593,10 @@ export class TerminalView extends LitElement {
               return
             }
             this.exitCode = code
-
-            if (!this.isAutoSaveEnabled) {
-              return
+            if (features.isOn(FeatureName.SignedIn, this.featureState$) && this.isAutoSaveEnabled) {
+              return this.#shareCellOutput(false)
             }
-
-            return this.#shareCellOutput(false)
+            return
           }
         }
       }),
@@ -993,18 +990,25 @@ export class TerminalView extends LitElement {
           () => {},
         )}
         ${when(
-          (this.exitCode === undefined || this.exitCode === 0 || !this.platformId) &&
-            !this.isDaggerOutput &&
-            features.isOn(FeatureName.Share, this.featureState$),
+          this.shouldRenderSaveButton(),
           () => {
-            return html` <action-button
+            return html`<save-button
               ?loading=${this.isLoading}
-              ?shareIcon="${!!this.platformId}"
-              ?saveIcon="${!this.platformId}"
-              text="${this.platformId ? 'Share' : 'Save'}"
+              ?signedIn=${features.isOn(FeatureName.SignedIn, this.featureState$)}
               @onClick="${this.#triggerShareCellOutput}"
             >
-            </action-button>`
+            </save-button>`
+          },
+          () => {},
+        )}
+        ${when(
+          this.shouldRenderShareButton(),
+          () => {
+            return html`<share-button
+              ?loading=${this.isLoading}
+              @onClick="${this.#triggerShareCellOutput}"
+            >
+            </share-button>`
           },
           () => {},
         )}
@@ -1015,7 +1019,7 @@ export class TerminalView extends LitElement {
             this.platformId &&
             !this.isDaggerOutput,
           () => {
-            return html` <action-button
+            return html`<action-button
               ?loading=${this.isCreatingEscalation}
               ?saveIcon="${true}"
               text="Escalate"
@@ -1029,7 +1033,7 @@ export class TerminalView extends LitElement {
         ${when(
           features.isOn(FeatureName.Escalate, this.featureState$) && this.escalationUrl,
           () => {
-            return html` <action-button
+            return html`<action-button
               ?saveIcon="${true}"
               text="Open Escalation"
               @onClick="${this.#triggerOpenEscalation}"
@@ -1041,7 +1045,7 @@ export class TerminalView extends LitElement {
         ${when(
           this.platformId && !this.isLoading,
           () => {
-            return html` <open-cell
+            return html`<open-cell
               ?disabled=${!this.isPlatformAuthEnabled}
               @onOpen="${this.#triggerOpenCellOutput}"
             ></open-cell>`
@@ -1078,6 +1082,16 @@ export class TerminalView extends LitElement {
         ),
       )
   }
+
+  shouldRenderSaveButton() {
+    const isExitCodeValid = this.exitCode === undefined || this.exitCode === 0
+    return !this.platformId && isExitCodeValid && !this.isDaggerOutput
+  }
+
+  shouldRenderShareButton() {
+    const isFeatureEnabled = features.isOn(FeatureName.Share, this.featureState$)
+    return this.platformId && isFeatureEnabled && this.isShareReady
+  }
 }
 
 function convertXTermDimensions(dimensions: ITerminalDimensions): TerminalDimensions

src/client/components/terminal/saveButton.ts

@@ -0,0 +1,35 @@
+import { LitElement, html } from 'lit'
+import { customElement, property } from 'lit/decorators.js'
+
+import './actionButton'
+
+@customElement('save-button')
+export class SaveButton extends LitElement {
+  @property({ type: Boolean, reflect: true })
+  loading: boolean = false
+
+  @property({ type: Boolean, reflect: true })
+  signedIn: boolean = false
+
+  private handleClick(e: Event) {
+    if (e.defaultPrevented) {
+      e.preventDefault()
+    }
+
+    this.dispatchEvent(new CustomEvent('onClick'))
+  }
+
+  render() {
+    let text = this.signedIn ? 'Save' : 'Save to Cloud'
+
+    return html`
+      <action-button
+        ?loading=${this.loading}
+        text="${text}"
+        ?saveIcon=${true}
+        @onClick="${this.handleClick}"
+      >
+      </action-button>
+    `
+  }
+}

src/client/components/terminal/shareButton.ts

@@ -0,0 +1,30 @@
+import { LitElement, html } from 'lit'
+import { customElement, property } from 'lit/decorators.js'
+
+import './actionButton'
+
+@customElement('share-button')
+export class ShareButton extends LitElement {
+  @property({ type: Boolean, reflect: true })
+  loading: boolean = false
+
+  private handleClick(e: Event) {
+    if (e.defaultPrevented) {
+      e.preventDefault()
+    }
+
+    this.dispatchEvent(new CustomEvent('onClick'))
+  }
+
+  render() {
+    return html`
+      <action-button
+        ?loading=${this.loading}
+        text="Share"
+        ?shareIcon=${true}
+        @onClick="${this.handleClick}"
+      >
+      </action-button>
+    `
+  }
+}

src/client/index.ts

@@ -136,13 +136,6 @@ export const activate: ActivationFunction<void> = (context) => {
             )
           }
 
-          if (payload.output.isSessionOutputsEnabled) {
-            terminalElement.setAttribute(
-              'isSessionOutputsEnabled',
-              payload.output.isSessionOutputsEnabled.toString(),
-            )
-          }
-
           if (payload.output.isPlatformAuthEnabled) {
             terminalElement.setAttribute(
               'isPlatformAuthEnabled',

src/constants.ts

@@ -200,7 +200,7 @@ export const NOTEBOOK_AUTOSAVE_ON = 'notebookAutoSaveOn'
 export const NOTEBOOK_LIFECYCLE_ID = 'notebookLifecycleId'
 export const NOTEBOOK_OUTPUTS_MASKED = 'notebookOutputsMasked'
 export const NOTEBOOK_MODE = 'notebookMode'
-export const NOTEBOOK_HAS_OUTPUTS = 'notebookHasRunmeOutputs'
+export const NOTEBOOK_PREVIEW_OUTPUTS = 'notebookPreviewRunmeOutputs'
 export const NOTEBOOK_RUN_WITH_PROMPTS = 'notebookRunWithPrompts'
 export const NOTEBOOK_AUTHOR_MODE_ON = 'notebookAuthorModeOn'
 

src/extension/cell.ts

@@ -251,17 +251,16 @@ export class NotebookCellOutputManager {
               ? ContextState.getKey(PLATFORM_USER_SIGNED_IN)
               : ContextState.getKey(GITHUB_USER_SIGNED_IN)
 
-            const isSessionOutputsEnabled = getSessionOutputs()
-
             const json: CellOutputPayload<OutputType.terminal> = {
               type: OutputType.terminal,
               output: {
                 'runme.dev/id': cellId,
                 content: stdoutBase64,
                 initialRows: terminalRows || terminalConfigurations.rows,
-                isAutoSaveEnabled: isSignedIn ? ContextState.getKey(NOTEBOOK_AUTOSAVE_ON) : false,
+                isAutoSaveEnabled: isSignedIn
+                  ? ContextState.getKey<boolean>(NOTEBOOK_AUTOSAVE_ON)
+                  : false,
                 isPlatformAuthEnabled: isPlatformAuthEnabled(),
-                isSessionOutputsEnabled,
                 isDaggerOutput: !!daggerOutput,
                 ...terminalConfigurations,
               },
@@ -525,6 +524,13 @@ export class NotebookCellOutputManager {
     })
   }
 
+  shouldSkipRefreshTerminal() {
+    const isSignedIn = features.isOnInContextState(FeatureName.SignedIn)
+    const isForceLogin = features.isOnInContextState(FeatureName.ForceLogin)
+
+    return !isSignedIn && isForceLogin
+  }
+
   /**
    * Syncs a stdout output item based on the active terminal
    *
@@ -538,15 +544,8 @@ export class NotebookCellOutputManager {
    *
    */
   async refreshTerminal(terminalState: ITerminalState | undefined): Promise<void> {
-    const isSignedIn = features.isOnInContextState(FeatureName.SignedIn)
-    const isForceLogin = features.isOnInContextState(FeatureName.ForceLogin)
-
-    const isAutoSaveOn = ContextState.getKey(NOTEBOOK_AUTOSAVE_ON)
-
-    if (!isSignedIn && !isAutoSaveOn) {
-      return Promise.resolve()
-    } else if (!isSignedIn && isForceLogin) {
-      return Promise.resolve()
+    if (this.shouldSkipRefreshTerminal()) {
+      return
     }
 
     await this.withLock(async () => {

src/extension/commands/index.ts

@@ -48,6 +48,7 @@ import {
   ClientMessages,
   TELEMETRY_EVENTS,
   RUNME_FRONTMATTER_PARSED,
+  NOTEBOOK_PREVIEW_OUTPUTS,
 } from '../../constants'
 import ContextState from '../contextState'
 import { createGist } from '../services/github/gist'
@@ -56,6 +57,7 @@ import { GetUserEnvironmentsDocument } from '../__generated-platform__/graphql'
 import { EnvironmentManager } from '../environment/manager'
 import features from '../features'
 import { insertCodeNotebookCell } from '../cell'
+import { GrpcSerializer, SerializerBase } from '../serializer'
 
 const log = getLogger('Commands')
 
@@ -308,6 +310,7 @@ export async function askNewRunnerSession(kernel: Kernel) {
     'OK',
   )
   if (action) {
+    await ContextState.addKey(NOTEBOOK_PREVIEW_OUTPUTS, false)
     await commands.executeCommand('workbench.action.files.save')
     await kernel.newRunnerEnvironment({})
     await commands.executeCommand('workbench.action.files.save')
@@ -324,7 +327,7 @@ export async function askAlternativeOutputsAction(
   metadata: { [key: string]: any },
 ): Promise<void> {
   const action = await window.showWarningMessage(
-    'Running Session Outputs from a previous notebook session is not supported.',
+    'Running Preview Outputs from a previous notebook session is not supported.',
     { modal: true },
     ASK_ALT_OUTPUTS_ACTION.ORIGINAL,
   )
@@ -591,3 +594,26 @@ export async function selectEnvironment(manager: EnvironmentManager) {
     )
   }
 }
+
+export function notebookSessionOutputs(kernel: Kernel, serializer: SerializerBase) {
+  return async (e: NotebookUiEvent) => {
+    const runnerEnv = kernel.getRunnerEnvironment()
+    const sessionId = runnerEnv?.getSessionId()
+    if (!e.ui || !sessionId) {
+      return
+    }
+
+    await ContextState.addKey(NOTEBOOK_PREVIEW_OUTPUTS, true)
+    const { notebookUri } = e.notebookEditor
+    const outputFilePath = GrpcSerializer.getOutputsUri(notebookUri, sessionId)
+
+    try {
+      await workspace.fs.stat(outputFilePath)
+    } catch (e) {
+      await commands.executeCommand('workbench.action.files.save')
+    }
+
+    await serializer.saveNotebookOutputs(notebookUri)
+    await openFileAsRunmeNotebook(outputFilePath)
+  }
+}