feat: line highlights on editor save, used for k edit apply errors

Fixes kubernetes-sigs#4795

packages/core/src/index.ts

@@ -146,7 +146,7 @@ export {
   Mode as MultiModalMode,
   MultiModalResponse
 } from './models/mmr/types'
-export { Editable, EditableSpec } from './models/editable'
+export { Editable, EditableSpec, SaveError } from './models/editable'
 export {
   Breadcrumb,
   NavResponse,

packages/core/src/models/editable.ts

@@ -13,21 +13,34 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 import { ToolbarText } from '../webapp/views/toolbar-text'
 
-export type Editable = { spec: EditableSpec }
+type Save = void | { noToolbarUpdate?: boolean; toolbarText?: ToolbarText }
+
+export interface SaveError extends Error {
+  revealLine?: number
+}
 
 export interface EditableSpec {
+  /** Should the editor be opened in read-only mode? Default: false */
   readOnly: boolean
+
+  /** Should we offer a Clear button? Default: false */
   clearable: boolean
+
+  /** Button and Controller to handle saves */
   save: {
     label: string
     onSave(data: string): Save | Promise<Save>
   }
+
+  /** Button and Controller to handle reverts */
   revert: {
     label?: string
     onRevert(): string | Promise<string>
   }
 }
 
-type Save = void | { noToolbarUpdate?: boolean; toolbarText?: ToolbarText }
+/** The Resource trait */
+export type Editable = { spec: EditableSpec }

plugins/plugin-client-common/src/components/Content/Editor/index.tsx

@@ -19,7 +19,16 @@ import { extname } from 'path'
 import { IDisposable, editor as Monaco, Range } from 'monaco-editor'
 
 import { File, isFile } from '@kui-shell/plugin-bash-like/fs'
-import { Button, REPL, StringContent, ToolbarText, ToolbarProps, MultiModalResponse, i18n } from '@kui-shell/core'
+import {
+  Button,
+  REPL,
+  SaveError,
+  StringContent,
+  ToolbarText,
+  ToolbarProps,
+  MultiModalResponse,
+  i18n
+} from '@kui-shell/core'
 
 import ClearButton from './ClearButton'
 import SaveFileButton from './SaveFileButton'
@@ -128,7 +137,14 @@ export default class Editor extends React.PureComponent<Props, State> {
   }
 
   private static onChange(props: Props, editor: Monaco.ICodeEditor) {
+    let currentDecorations: string[]
+
     return () => {
+      if (currentDecorations) {
+        editor.deltaDecorations(currentDecorations, [])
+        currentDecorations = undefined
+      }
+
       const clearable = Editor.isClearable(props)
 
       const buttons: Button[] = []
@@ -158,7 +174,23 @@ export default class Editor extends React.PureComponent<Props, State> {
                   !clearable ? undefined : [ClearButton(editor)]
                 )
               }
-            } catch (err) {
+            } catch (error) {
+              const err = error as SaveError
+
+              if (err.revealLine) {
+                editor.revealLineInCenter(err.revealLine)
+                currentDecorations = editor.deltaDecorations(currentDecorations || [], [
+                  {
+                    range: new Range(err.revealLine, 1, err.revealLine, 1),
+                    options: {
+                      isWholeLine: true,
+                      className: 'kui--editor-line-highlight',
+                      glyphMarginClassName: 'kui--editor-line-glyph'
+                    }
+                  }
+                ])
+              }
+
               const alert = {
                 type: 'warning' as const,
                 text: strings('isModified'),
@@ -171,7 +203,7 @@ export default class Editor extends React.PureComponent<Props, State> {
                 ]
               }
 
-              props.willUpdateToolbar(alert, undefined, undefined)
+              props.willUpdateToolbar(alert, buttons, undefined)
             }
           }
         })

plugins/plugin-client-common/web/scss/components/Editor/Editor.scss

@@ -139,3 +139,13 @@ body[kui-theme-style] .monaco-editor .selected-text {
   background: var(--color-sidecar-toolbar-background);
   color: var(--color-sidecar-toolbar-foreground);
 }
+
+/** Error desiginations */
+.kui--editor-line-glyph {
+  background-color: var(--color-error);
+}
+
+/** Highlight designations */
+.kui--editor-line-highlight {
+  background-color: var(--color-base03);
+}

plugins/plugin-kubectl/src/controller/kubectl/edit.ts

@@ -22,6 +22,7 @@ import {
   ExecOptions,
   i18n,
   EditableSpec,
+  SaveError,
   isStringWithOptionalContentType
 } from '@kui-shell/core'
 
@@ -52,6 +53,61 @@ export function isEditable(resource: KubeResource) {
   )
 }
 
+/**
+ * Reformat the apply error.
+ *
+ * @param tmp the temporary file we used to stage the apply -f ${tmp}
+ * @param data the raw data we attempted to apply
+ *
+ */
+function reportErrorToUser(tmp: string, data: string, err: Error) {
+  console.error('error in apply for edit', err)
+
+  // was this a validation error?
+  const msg = err.message.match(/ValidationError\(.*\): ([^]+) in/)
+  if (msg && msg.length === 2) {
+    const unknownField = err.message.match(/unknown field "(.*)"/)
+    const error: SaveError = new Error(msg[1])
+    if (unknownField) {
+      const regexp = new RegExp(`${unknownField[1]}:`)
+      const lineNumber = data.split(/\n/).findIndex(line => regexp.test(line))
+      if (lineNumber >= 0) {
+        // monaco indexes from 1
+        error.revealLine = lineNumber + 1
+      }
+    }
+    throw error
+  } else {
+    // maybe this was a syntax error?
+    const msg = err.message.match(/error parsing.*(line .*)/)
+    if (msg && msg.length === 2) {
+      const hasLineNumber = err.message.match(/line (\d+):/)
+      const error: SaveError = new Error(msg[1])
+      if (hasLineNumber) {
+        // not sure why, but this line number is off by + 1
+        error.revealLine = parseInt(hasLineNumber[1]) - 1
+      }
+      throw error
+    } else {
+      // maybe this was a conflict error
+      if (err.message.indexOf('Error from server (Conflict)') !== -1) {
+        const errorForFile = `for: "${tmp}":`
+        const forFile = err.message.indexOf(errorForFile)
+        const messageForFile = err.message.substring(forFile).replace(errorForFile, '')
+        throw new Error(messageForFile)
+      }
+      // hmm, some other random error
+      const msg = err.message.replace(tmp, '')
+      const newLines = msg.split('\n')
+      if (newLines[0].charAt(newLines[0].length - 2) === ':') {
+        throw new Error(newLines.slice(0, 2).join('\n'))
+      } else {
+        throw new Error(newLines[0])
+      }
+    }
+  }
+}
+
 export function editSpec(
   cmd: string,
   namespace: string,
@@ -75,44 +131,18 @@ export function editSpec(
           parsedOptions: { n: namespace, f: tmp }
         })
 
-        // execute the apply command, making sure to report any
-        // validation or parse errors to the user
-        await doExecWithStdout(applyArgs, undefined, cmd).catch(err => {
-          console.error('error in apply for edit', err)
-
-          // was this a validation error?
-          const msg = err.message.match(/ValidationError\(.*\): ([^]+) in/)
-          if (msg && msg.length === 2) {
-            throw new Error(msg[1])
-          } else {
-            // maybe this was a syntax error?
-            const msg = err.message.match(/error parsing.*(line .*)/)
-            if (msg && msg.length === 2) {
-              throw new Error(msg[1])
-            } else {
-              // maybe this was a conflict error
-              if (err.message.indexOf('Error from server (Conflict)') !== -1) {
-                const errorForFile = `for: "${tmp}":`
-                const forFile = err.message.indexOf(errorForFile)
-                const messageForFile = err.message.substring(forFile).replace(errorForFile, '')
-                throw new Error(messageForFile)
-              }
-              // hmm, some other random error
-              const msg = err.message.replace(tmp, '')
-              const newLines = msg.split('\n')
-              if (newLines[0].charAt(newLines[0].length - 2) === ':') {
-                throw new Error(newLines.slice(0, 2).join('\n'))
-              } else {
-                throw new Error(newLines[0])
-              }
-            }
-          }
-        })
+        try {
+          // execute the apply command, making sure to report any
+          // validation or parse errors to the user
+          await doExecWithStdout(applyArgs, undefined, cmd).catch(reportErrorToUser.bind(undefined, tmp, data))
 
-        // to show the updated resource after apply,
-        // we re-execute the original edit command after applying the changes.
-        // `partOfApply` here is used to signify this execution is part of a chain of controller
-        await args.REPL.pexec(args.command, { echo: false, data: { partOfApply: true } })
+          // to show the updated resource after apply,
+          // we re-execute the original edit command after applying the changes.
+          // `partOfApply` here is used to signify this execution is part of a chain of controller
+          await args.REPL.pexec(args.command, { echo: false, data: { partOfApply: true } })
+        } finally {
+          args.REPL.qexec(`rm -f ${tmp}`)
+        }
 
         return {
           // disable editor's auto toolbar update,
@@ -198,6 +228,11 @@ function isEditAfterApply(options: ExecOptions): options is EditAfterApply {
   return opts && opts.data && opts.data.partOfApply !== undefined
 }
 
+/**
+ * Variant of `resource` enhanced with an `Editable` impl that saves
+ * via kubectl apply.
+ *
+ */
 export async function editable(
   cmd: string,
   args: Arguments<KubeOptions>,
@@ -221,6 +256,7 @@ export async function editable(
   return view
 }
 
+/** Variant of `resource` that shows the given `defaultMode` upon open */
 function showingMode(defaultMode: string, resource: MultiModalResponse) {
   return Object.assign(resource, { defaultMode })
 }
@@ -239,7 +275,7 @@ export function register(registrar: Registrar, cmd: string) {
   registrar.listen(`/${commandPrefix}/${cmd}/edit`, doEdit.bind(undefined, cmd), editFlags(cmd))
 }
 
-export default (registrar: Registrar) => {
+export default function registerForKubectl(registrar: Registrar) {
   register(registrar, 'kubectl')
   register(registrar, 'k')
 }

plugins/plugin-kubectl/src/test/k8s1/edit.ts

@@ -93,15 +93,15 @@ commands.forEach(command => {
           // an error state and the garbage text had better appear in the toolbar text
           await SidecarExpect.toolbarAlert({ type: 'error', text: expectedError || garbage, exact: false })(this.app)
 
+          // expect line number to be highlighted, and for that line to be visible
+          await this.app.client.waitForVisible(`${Selectors.SIDECAR_TAB_CONTENT} .kui--editor-line-highlight`)
+
           if (revert) {
-            await this.app.client.click(lineSelector)
-            await new Promise(resolve => setTimeout(resolve, 2000))
-            await this.app.client.keys(
-              where === Keys.Home
-                ? `${where}${Keys.DELETE.repeat(garbage.length)}`
-                : `${where}${Keys.BACKSPACE.repeat(garbage.length)}`
-            )
-            await new Promise(resolve => setTimeout(resolve, 2000))
+            await this.app.client.click(Selectors.SIDECAR_MODE_BUTTON('Revert'))
+            await this.app.client.waitUntil(async () => {
+              const revertedText = await Util.getValueFromMonaco(this.app)
+              return revertedText === actualText
+            }, CLI.waitTimeout)
           }
         } catch (err) {
           await Common.oops(this, true)(err)