From 264ca58c46f6a660e190b537f63dacfc3fc8d513 Mon Sep 17 00:00:00 2001 From: Jos de Jong Date: Thu, 29 Feb 2024 19:52:20 +0100 Subject: [PATCH] feat: do not trigger onChange on programmatic changes (#318) --- .../modes/tablemode/TableMode.svelte | 27 +++------ .../components/modes/textmode/TextMode.svelte | 56 +++++++++++-------- .../components/modes/treemode/TreeMode.svelte | 36 ++---------- 3 files changed, 47 insertions(+), 72 deletions(-) diff --git a/src/lib/components/modes/tablemode/TableMode.svelte b/src/lib/components/modes/tablemode/TableMode.svelte index 1ee192a6..ca7fbe60 100644 --- a/src/lib/components/modes/tablemode/TableMode.svelte +++ b/src/lib/components/modes/tablemode/TableMode.svelte @@ -419,13 +419,6 @@ previousText, previousTextIsRepaired }) - - // we could work out a patchResult, or use patch(), but only when the previous and new - // contents are both json and not text. We go for simplicity and consistency here and - // let the function applyExternalContent _not_ return a patchResult ever. - const patchResult = null - - emitOnChange(previousContent, patchResult) } function applyExternalSelection(externalSelection: JSONEditorSelection | null) { @@ -580,7 +573,6 @@ throw new Error('Cannot apply patch: no JSON') } - const previousContent: Content = { json } const previousJson = json const previousState = documentState const previousTextIsRepaired = textIsRepaired @@ -638,8 +630,6 @@ redo: operations } - emitOnChange(previousContent, patchResult) - return patchResult } @@ -647,17 +637,14 @@ operations: JSONPatchDocument, afterPatch?: AfterPatchCallback ): JSONPatchResult { - if (readOnly) { - // this should never happen in practice - return { - json, - previousJson: json, - redo: [], - undo: [] - } - } + debug('handlePatch', operations, afterPatch) + + const previousContent = { json, text } + const patchResult = patch(operations, afterPatch) + + emitOnChange(previousContent, patchResult) - return patch(operations, afterPatch) + return patchResult } function emitOnChange(previousContent: Content, patchResult: JSONPatchResult | null) { diff --git a/src/lib/components/modes/textmode/TextMode.svelte b/src/lib/components/modes/textmode/TextMode.svelte index 3ebad394..0938289c 100644 --- a/src/lib/components/modes/textmode/TextMode.svelte +++ b/src/lib/components/modes/textmode/TextMode.svelte @@ -173,7 +173,7 @@ escapeUnicodeCharacters }) - $: setCodeMirrorContent(externalContent) + $: setCodeMirrorContent(externalContent, false, false) $: applyExternalSelection(externalSelection) $: updateLinter(validator) $: updateIndentation(indentation) @@ -250,14 +250,20 @@ }) export function patch(operations: JSONPatchDocument): JSONPatchResult { - debug('patch', operations) + return handlePatch(operations, false) + } + + export function handlePatch(operations: JSONPatchDocument, emitChange: boolean): JSONPatchResult { + debug('handlePatch', operations, emitChange) const previousJson = parser.parse(text) const updatedJson = immutableJSONPatch(previousJson, operations) const undo = revertJSONPatch(previousJson, operations) - setCodeMirrorContent({ + const updatedContent = { text: parser.stringify(updatedJson, null, indentation) as string - }) + } + + setCodeMirrorContent(updatedContent, emitChange, false) return { json: updatedJson, @@ -275,11 +281,12 @@ } try { - const json = parser.parse(text) - setCodeMirrorContent({ - text: parser.stringify(json, null, indentation) as string - }) - askToFormat = true + const updatedJson = parser.parse(text) + const updatedContent = { + text: parser.stringify(updatedJson, null, indentation) as string + } + + setCodeMirrorContent(updatedContent, true, false) return true } catch (err) { @@ -297,11 +304,12 @@ } try { - const json = parser.parse(text) - setCodeMirrorContent({ - text: parser.stringify(json) as string - }) - askToFormat = false + const updatedJson = parser.parse(text) + const updatedContent = { + text: parser.stringify(updatedJson) as string + } + + setCodeMirrorContent(updatedContent, true, false) return true } catch (err) { @@ -319,9 +327,12 @@ } try { - setCodeMirrorContent({ + const updatedContent = { text: jsonrepair(text) - }) + } + + setCodeMirrorContent(updatedContent, true, false) + jsonStatus = JSON_STATUS_VALID jsonParseError = null } catch (err) { @@ -345,7 +356,7 @@ rootPath: [], onSort: async ({ operations }) => { debug('onSort', operations) - patch(operations) + handlePatch(operations, true) }, onClose: () => { modalOpen = false @@ -384,7 +395,7 @@ }) } else { debug('onTransform', operations) - patch(operations) + handlePatch(operations, true) } }, onClose: () => { @@ -445,7 +456,7 @@ function handleAcceptTooLarge() { acceptTooLarge = true - setCodeMirrorContent(externalContent, true) + setCodeMirrorContent(externalContent, true, true) } function handleSwitchToTreeMode() { @@ -671,12 +682,12 @@ } } - function setCodeMirrorContent(newContent: Content, forceUpdate = false) { + function setCodeMirrorContent(newContent: Content, emitChange: boolean, forceUpdate: boolean) { const newText = getText(newContent, indentation, parser) const isChanged = !isEqual(newContent, content) const previousContent = content - debug('setCodeMirrorContent', { isChanged, forceUpdate }) + debug('setCodeMirrorContent', { isChanged, emitChange, forceUpdate }) if (!codeMirrorView || (!isChanged && !forceUpdate)) { return @@ -698,7 +709,8 @@ } updateCanUndoRedo() - if (isChanged) { + + if (isChanged && emitChange) { emitOnChange(content, previousContent) } } diff --git a/src/lib/components/modes/treemode/TreeMode.svelte b/src/lib/components/modes/treemode/TreeMode.svelte index ab42838d..d2314f3d 100644 --- a/src/lib/components/modes/treemode/TreeMode.svelte +++ b/src/lib/components/modes/treemode/TreeMode.svelte @@ -533,14 +533,6 @@ previousText, previousTextIsRepaired }) - - // we could work out a patchResult, or use patch(), but only when the previous and new - // contents are both json and not text. We go for simplicity and consistency here and - // let the functions applyExternalJson and applyExternalText _not_ return - // a patchResult ever. - const patchResult = null - - emitOnChange(previousContent, patchResult) } function applyExternalText(updatedText: string | undefined) { @@ -597,14 +589,6 @@ previousText, previousTextIsRepaired }) - - // we could work out a patchResult, or use patch(), but only when the previous and new - // contents are both json and not text. We go for simplicity and consistency here and - // let the functions applyExternalJson and applyExternalText _not_ return - // a patchResult ever. - const patchResult = null - - emitOnChange(previousContent, patchResult) } function applyExternalSelection(externalSelection: JSONEditorSelection | null) { @@ -739,7 +723,6 @@ throw new Error('Cannot apply patch: no JSON') } - const previousContent = { json, text } const previousJson = json const previousState = documentState const previousText = text @@ -800,8 +783,6 @@ redo: operations } - emitOnChange(previousContent, patchResult) - return patchResult } @@ -1451,19 +1432,14 @@ operations: JSONPatchDocument, afterPatch?: AfterPatchCallback ): JSONPatchResult { - if (readOnly) { - // this should never happen in practice - return { - json, - previousJson: json, - undo: [], - redo: [] - } - } - debug('handlePatch', operations, afterPatch) - return patch(operations, afterPatch) + const previousContent = { json, text } + const patchResult = patch(operations, afterPatch) + + emitOnChange(previousContent, patchResult) + + return patchResult } function handleReplaceJson(updatedJson: unknown, afterPatch?: AfterPatchCallback) {