From 37c02d148468d5d2248aec012cf0fa451a9458be Mon Sep 17 00:00:00 2001 From: Yichao Yu Date: Thu, 3 Feb 2022 07:06:05 -0500 Subject: [PATCH] Include the auto layout result in the previous edit event This should make the saved result more consistent, and could also be useful if digitaljs starts to generate auto layout for other edits (e.g. if something changes the text size). --- src/document.mjs | 43 +++++++++++++++++++++++++++++++++++++++++-- view/main.mjs | 30 +++++++++++++++++++----------- 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/document.mjs b/src/document.mjs index d43b79e..932fc7a 100644 --- a/src/document.mjs +++ b/src/document.mjs @@ -24,6 +24,7 @@ export class Document { #extra_data = {} #synth_options = { ...default_synth_options } #circuit = { devices: {}, connectors: [], subcircuits: {} } + #last_circuit_changed #tick = 0 #iopanelViews = [] @@ -153,6 +154,7 @@ export class Document { this.#circuit[fld] = v; delete data[fld]; } + this.#last_circuit_changed = undefined; this.#extra_data = data; } #toBackup() { @@ -217,7 +219,9 @@ export class Document { // Edits #createEdit(before, after, label, cb) { if (_.isEqual(before, after)) - return; + return false; + // We must not copy `after` here since the caller (in particular #circuitEdit) + // may mutate the object to merge in changes later. this.#documentEdited.fire({ document: this, label: label, @@ -228,6 +232,7 @@ export class Document { cb(before); }, }); + return true; } // Actions @@ -256,10 +261,16 @@ export class Document { #circuitEdit(after, label) { const before = this.#circuit; this.#circuit = after; - this.#createEdit(before, after, label, (circuit) => { + const changed = this.#createEdit(before, after, label, (circuit) => { this.#circuit = circuit; + this.#last_circuit_changed = undefined; this.#circuitUpdated.fire(false); }); + if (!changed) { + this.#last_circuit_changed = undefined; + return; + } + this.#last_circuit_changed = after; } async doSynth() { // Load a snapshot of the options up front @@ -330,6 +341,31 @@ export class Document { } this.#circuitEdit(message.circuit, label); } + #processAutoLayout(message) { + // If some user action triggers the automatic layout of the circuit, + // we want to merge the change of the layout to the edit that corresponds + // to that action, which we'll assume be the previous edit of the ciruit. + // There are a few cases that we need to be careful though, + // 1. we don't want to create a new edit just for the auto layout + // since it'll make undo basically a no-op (it will trigger another auto layout + // and get us back to exactly where we started) + // and might confuse the vscode history management. + // This means that if we don't have an edit to merge with, + // we should not generate a new edit + // 2. we need to ignore the potential auto layout event after load/undo/redo/revert + // since those should set the document to a state that should be clean + // unless the user does something explicity. + // For these reasons, we'll clear the last circuit change if the edit was a no-op + // and after load/undo/redo/revert. + if (!this.#last_circuit_changed) { + this.#circuit = message.circuit; + return; + } + for (const key in this.#last_circuit_changed) + delete this.#last_circuit_changed[key]; + Object.assign(this.#last_circuit_changed, message.circuit); + this.#circuit = this.#last_circuit_changed; + } // Messages #processIOPanelMessage(message) { @@ -359,6 +395,9 @@ export class Document { case 'updatecircuit': this.#updateCircuit(message); return; + case 'autolayout': + this.#processAutoLayout(message); + return; case 'tick': this.tick = message.tick; return; diff --git a/view/main.mjs b/view/main.mjs index 66a95fa..0e7323a 100644 --- a/view/main.mjs +++ b/view/main.mjs @@ -407,24 +407,32 @@ class DigitalJS { }; // The layout actually uses display information (i.e. the text widths of the labels) // so we can't really do it well on the host side - // (it also means that we can't really guarantee portability). - // However, we still don't want to treat automatic layout changes as - // user edits so for now we'll just ignore all of them. + // (it also means that we can't really guarantee portability) + // and the circuit we loaded for the first time will have the layout information + // applied. + // However, we still don't want to treat these automatic layout changes as + // user edits so we'll send them to the user in a different kind of message + // to treat them as part of the previous edit. // - // This creates an minor inconsistency in the result that the initial circuit saved - // won't have layout info in it but when the user edited the circuit graphically - // it would. - // If the digitaljs library ever re-layout the circuit based on the changes in the text, - // etc, it should hopefully be done in a way that keeps the loading result consistent - // (i.e. if the pre-auto-layout circuit is saved, the load result should be identical - // assuming the same layout parameter is used, or in another word, - // the loading code should reapply layout adjustment) + // Note that since the auto layout is done lazily, + // unless the user opens all the subcircuits, the initial circuit saved + // might still not have all the layout info. let in_layout = false; this.circuit = new digitaljs.Circuit(data, circuit_opts); this.circuit.listenTo(this.circuit._graph, 'elkjs:layout_start', (ele) => { + const old_info = this.#change_tracker.info; + // Flush the changes before the layout starts + if (old_info) { + this.#change_tracker.clear(); + vscode.postMessage({ command: "updatecircuit", + circuit: this.circuit.toJSON(), + type: old_info.type, + ele_type: old_info.ele_type }); + } in_layout = true; }); this.circuit.listenTo(this.circuit._graph, 'elkjs:layout_end', (ele) => { + vscode.postMessage({ command: "autolayout", circuit: this.circuit.toJSON() }); in_layout = false; }); this.circuit.listenTo(this.circuit._graph, 'change:position', (ele) => {