diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7cfa3417a9a..6aa88cee96d 100644 --- a/packages/core/src/index.ts +++ b/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, diff --git a/packages/core/src/models/editable.ts b/packages/core/src/models/editable.ts index 50435dc77b5..e07302cf4cb 100644 --- a/packages/core/src/models/editable.ts +++ b/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 } + + /** Button and Controller to handle reverts */ revert: { label?: string onRevert(): string | Promise } } -type Save = void | { noToolbarUpdate?: boolean; toolbarText?: ToolbarText } +/** The Resource trait */ +export type Editable = { spec: EditableSpec } diff --git a/plugins/plugin-client-common/src/components/Content/Editor/index.tsx b/plugins/plugin-client-common/src/components/Content/Editor/index.tsx index c8cce27ee95..ecc037ee8ea 100644 --- a/plugins/plugin-client-common/src/components/Content/Editor/index.tsx +++ b/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 { } 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 { !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.willUpdateToolbar(alert, undefined, undefined) + props.willUpdateToolbar(alert, buttons, undefined) } } }) diff --git a/plugins/plugin-client-common/web/scss/components/Editor/Editor.scss b/plugins/plugin-client-common/web/scss/components/Editor/Editor.scss index e53dd07709d..e9c0e938944 100644 --- a/plugins/plugin-client-common/web/scss/components/Editor/Editor.scss +++ b/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); +} diff --git a/plugins/plugin-kubectl/src/controller/kubectl/edit.ts b/plugins/plugin-kubectl/src/controller/kubectl/edit.ts index 8e5e3ddf6c5..09c309a6608 100644 --- a/plugins/plugin-kubectl/src/controller/kubectl/edit.ts +++ b/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, @@ -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') } diff --git a/plugins/plugin-kubectl/src/test/k8s1/edit.ts b/plugins/plugin-kubectl/src/test/k8s1/edit.ts index d3ff4df6a16..32723cfd145 100644 --- a/plugins/plugin-kubectl/src/test/k8s1/edit.ts +++ b/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)