Skip to content

Commit

Permalink
Include the auto layout result in the previous edit event
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
yuyichao committed Feb 3, 2022
1 parent a040ae5 commit 37c02d1
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 13 deletions.
43 changes: 41 additions & 2 deletions src/document.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export class Document {
#extra_data = {}
#synth_options = { ...default_synth_options }
#circuit = { devices: {}, connectors: [], subcircuits: {} }
#last_circuit_changed

#tick = 0
#iopanelViews = []
Expand Down Expand Up @@ -153,6 +154,7 @@ export class Document {
this.#circuit[fld] = v;
delete data[fld];
}
this.#last_circuit_changed = undefined;
this.#extra_data = data;
}
#toBackup() {
Expand Down Expand Up @@ -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,
Expand All @@ -228,6 +232,7 @@ export class Document {
cb(before);
},
});
return true;
}

// Actions
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
30 changes: 19 additions & 11 deletions view/main.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down

0 comments on commit 37c02d1

Please sign in to comment.