Repository navigation
Host the compressed dictionary editor on view's binary engine (6b V2) - #169
Merged
Merged
Conversation
…s last
A test of today's behaviour, ahead of the binary editor's move onto view's
engine: two saves whose writes land out of order. VS Code cancels a save's
token when it starts another save of the same document, and ignores the
first one's result, but the first save's write goes on and can land after
the newer save's. The file then holds the older bytes, and the next save
is refused as a fault in the editor ("Refusing to save: entry "gravity"
would be written with bytes other than the file's"), though the user
only saved twice.
The provider gains writeUserFile as a property, as askSaveConflict is
one, so that the suite can hold the first save's write as a slow disk
would. It defaults to diskFollow's.
v1.2.2 and v1.2.3 add what the compressed dictionary editor needs to move
onto view's binary engine, on ./host only: the engine's
trailerAfterDelivery, opened(view) matched by identity in both engines,
a seam for this extension's integration suites (BinaryEngine.forTests:
setChunk, pushEdit, resetParts, views and modified), and closing a
document closes only its real tables. Nothing changes for apps.
The lockfile resolves the tag to its commit, 618edfc (the ^{} line of
ls-remote), at version 1.2.3, with no integrity field; view's core peer
is still ^1.54.0. viewPin.test.ts moves with it.
BinarySlddEditorProvider is now a host over data-explorer-view's
BinaryEngine: the parsed package, the tree, its edits, the Status
column's baseline, the save gate, the paints and every gesture are the
engine's, lifted from this file. What stays is VS Code's end of each
hook and what is this extension's own: the disk (its stamp, a save
conflict, the watcher), the save's sequence, the hot-exit backup, the
webview and the messages every table answers alike.
- binaryDocument.ts: the document, the engine with VS Code's end of its
hooks (the edit stack and VS Code's Undo and Redo, view's
EditStackIndex for its position and save point, notices, the
registers), and the engine's forTests seam that the integration
suites read and write through (chunkXml, pushEdit, the views, the
Status column's set).
- engineTable.ts: an editor whose post() did not await its rows
(awaitRows: false) posts them and what follows them in one run, and
answers ready at once, as this editor's post() did. A setRows of the
editor's own spelling (binaryRows) replaces the MAT-file editor's.
- diskConflict.ts: EditStackMirror is view's EditStackIndex, the same
rule, under its old name until the suites are rebalanced.
One behaviour changes, on purpose: a save whose write lands after a
newer save's leaves the file holding the older bytes, and the next save
was refused as a fault in the editor ("Refusing to save: entry ... would
be written with bytes other than the file's"). The engine gives the
newer saves' edits back when the older save lands, so the next save
writes the newest edits. binarySlddSaveOrder.test.ts, which pinned the
refusal, now pins that.
Tests that follow the code:
- binarySlddSaveGate.test.ts reads a save's baseline from the engine
(modified(root), which throws if no baseline was taken) where it read
slddBaseline's computeModified, which the engine no longer feeds and
so would answer [] whatever the save did. Names and expected values
are unchanged.
- binarySaveGate.test.ts checks the save's own half of the gate in the
engine's beginSave, and that writeDocument calls it before its first
await.
- messageDispatch, multiSelectInvariants and slddParts read the
provider's source; they follow it to engineTable's own setRows, the
engine table's paste and drop, and jsonEntryScan.ts alone.
- editStackMirror.test.ts: EditStackMirror is EditStackIndex.
…s read them The compressed dictionary editor's setRows builder lived in its provider, which imports vscode, so engineTable.test.ts pinned a copy of it: a key reordered or added in the provider's literal would have passed that test. Both builders, matRows and binaryRows, are now exported by engineTable.ts (vscode-free), each provider hands its own over as `setRows` (no longer optional), and the test posts through the real ones and pins their keys in order. The literals are unchanged, key for key. messageDispatch.test.ts's "every setRows says which document it is describing" reads, with a provider that calls engineTable, the one builder it names in engineTable.ts. Still four posts, each with docUri.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 6b, V2: the compressed dictionary (binary .sldd) editor becomes a host over view's binary engine (
BinaryEngine,data-explorer-view/host, view v1.2.3). The parsed package, the tree, its edits, the Status column's baseline, the save gate, the paints and every gesture are the engine's, lifted in 6a from this file at c73da3c (unchanged since, up to main). This extension keeps VS Code's end of each hook and what is its own: the disk (stamp, save conflict, watcher), the save's sequence (saveSettled), the hot-exit backup, the webview, and the messages every table answers alike.One behaviour change, named (D44); nothing else is intended to change. The P4 runners give the same saves as main's V0c run, file by file, and the same answers. The screenshot and posted-message A/B is identical. The integration suites are unchanged and pass, apart from the D44 test this PR adds and the two re-pointed assertions below.
Commits:
binarySlddSaveOrder.test.ts, of today's behaviour, and the seam it needs (the provider'swriteUserFileas a property, asaskSaveConflictis one; it defaults to diskFollow's). It passes on main's provider in VS Code 1.144.0.trailerAfterDelivery;opened(view)matched to its paint by identity in both engines (review168's race);BinaryEngine.forTests, a test seam for this extension's suites only; and a dispose that skips a suite's stand-in views.matRowsandbinaryRows, are exported byengineTable.ts(vscode-free). Each provider hands its own builder over assetRows, which is no longer optional, andengineTable.test.tsposts through the real ones instead of a copy. The literals are unchanged, key for key.D44: the one behaviour change
saveCustomDocumentwith a token the test cancels, as VS Code does when it starts a newer save (review T2). Driving VS Code's own save pipeline instead (workspace.save(uri)twice with the first write held, then the tab'sisDirty) is not possible without a production hook.activateexposes nothing. The suites' providers are their own instances in the test bundle, not the registered one. A real tab's edits arrive only from its webview, which the extension host cannot script. So the suite pins the provider's behaviour, and VS Code's overlap (it cancels the first save's token and ignores its result) rests onsettledSave.ts's note, measured against VS Code 1.143's workbench.dataExplorer.tableView) on a compressed .sldd, both in a second editor group and in a split. Both times VS Code refused it ("File seems to be binary and cannot be opened as text"), and only the binary tab stayed open. The JSON tab'sunregisterSourceDeleter(uri)on close behaves as before.What moves where
BinarySlddEditorProvider.tsgoes from 1,803 lines to 321. It keepsopenCustomDocument(the read, the misroute guard, the watcher),resolveCustomEditor, the save, Save As, revert and backup. The webview's messages still go throughdispatchCommonand the provider's own dispatch chain (no try around it, as before), whose branches now call the engine's table.binaryDocument.ts(new) is the document: the engine plus VS Code's end of its hooks, asmatDocument.tsis for the MAT-file editor.onDidChangeCustomDocument, with VS Code'sundo/redocommands.EditStackIndex.hubRegisters(), shared with the MAT and JSON editors until V4.misrouted).chunkXml,pushEdit,resetParts,viewsandmodified(node)forward toBinaryEngine.forTests;zipMetareadsengine.parts. No bracket access.engineTable.tsgains the binary editor's timing and spelling:awaitRows: false: this editor'spost()never awaited its rows, so the rows' post (throughpostOrReport), the trailer anddrainNavigateSelectgo out in one run, andreadyis answered in the run that asked, by a repaint;setRows: each editor's own literal, exported fromengineTable.ts(binaryRows:{type, docUri, rows, columns, columnLabels, columnGroups, editable, warnings}, nonoticeoradd;matRows, V1's), which the provider hands over;opened(view): both relays now pass the delivered view, so the engine pairs a trailer with its own paint.diskConflict.ts:EditStackMirroris nowexport { EditStackIndex as EditStackMirror }from view/host, the same rule line for line, so diskConflict.test.ts and settledSave.test.ts stay unchanged. The name goes in V6.src/, but 10 unit test files import it; it goes with the structuralEdit family in V4), slddBaseline (the JSON editor), transfer, clipboardAction, clipMarks, refusal, editTextRequest, entryOps and structuralEdit.Test changes, and why
The vscode integration suites are unchanged except for these two files:
test-integration/suite/binarySlddSaveOrder.test.ts(new in commit 1, flipped in commit 3) is D44's test, above. Its name says the behaviour, so it changes with it: "… and the next save is refused as a fault in the editor" becomes "… and the next save writes the newest edits".test-integration/suite/binarySlddSaveGate.test.ts, two assertions (lines 178 and 228). Both read the save's baseline withslddBaseline.computeModified(uri, root). The engine keeps its own baseline and no longer feeds that map, so the call would answer[]whatever the save did: a vacuous pass. Both now read the engine's (doc.modified(root), which forwards toBinaryEngine.forTests.modified; it throws if no baseline was ever taken). The test names, the expected[]and the messages are unchanged.reBaseline'sthis.baseline.capture(node)), :228 fails with "No baseline has been taken for binary_save_race_copy.sldd." (its test opens no panel, so the save's baseline is the only one), and the suite's other 3 tests pass. Put back, the bundles are byte-identical to before. The old read could not have caught it: nothing feeds slddBaseline's map for this file any more, andcomputeModifiedanswers[]for a file it has no baseline for.chunkXml,zipMeta,srcId,views,repaintAll,pushEdit,resetParts) reaches the engine through the shell, andcomputeModifiedis left only in a comment of twoTabBaseline (the JSON editor's case).Unit tests (vitest) that read this extension's source follow the code there, with their names kept unless said:
test/binarySaveGate.test.ts, the describe "writeTo — the provider's own half of the save gate" becomes "the save's own half of the gate: the engine's beginSave, before the save's first await". Its two tests keep their names. "refuses a chunk that does not read back, in the words the user is shown" runs the engine (forTests.setChunkof an unreadable chunk, thenbeginSavethrows "Refusing to save: the document did not re-parse (…)."), where it read the literal from the provider's source. "reads the chunk, gates it and zips it with no await between" checks thatbeginSaveis one synchronous call and thatwriteDocumentcalls it before its first await.test/messageDispatch.test.ts, "every setRows says which document it is describing": a provider that callsengineTableis read with the one builder it names (setRows: binaryRows) fromengineTable.ts, where both literals are now spelled. Still 4 posts, each withdocUri.test/multiSelectInvariants.test.ts, " pastes and drops through runTransfer": a provider on an engine is checked to hand paste and drop to the engine's table (which runs view's runTransfer, pinned in view); the JSON editor's check is unchanged.test/slddParts.test.ts: "and the two files that navigate the part take the names from core" becomes "and the file that navigates the part takes the names from core", forjsonEntryScan.tsonly. The binary editor no longer navigates the package; the engine reads and writes it through core'sreadBinarySlddPackage/writeBinarySlddPackage.vitest.config.tsaddssrc/host/binaryDocument.tsto the coverage exclude list (it importsvscode), as V1 did for matDocument.ts.Inventory
engineTable.test.ts(2), the binary editor's mode: a paint posts the rows, then in the same run what follows them (sectionRules, clipboardState), then the drain, with the keys of the realbinaryRows, in order; rows it cannot deliver say "Failed to show" after what follows them, as the unawaited post did;editStackMirror.test.ts(1):EditStackMirroris view'sEditStackIndex;integrationSleeps.test.ts(1, generated per suite file): binarySlddSaveOrder.test.ts sleeps for no fixed time of its own.Gate
npm run verify: typecheck (bench included), both builds, 138 files and 2,227 tests, and check:leak.npm run test:integration(with the runners' environment set): 0 failing in either run. With the MAT and gesture runners set: 461 passing, 3 pending. WitheditorSaveSweepset: 460 passing, 4 pending. Each run's pending tests are the runners not set in it, plus the DEX_GATE_FIXTURES live gate (pending on main too). Both add up to 464 tests: the 463 of main's V0c run plus the D44 test.vsce ls: the same 136 files as main.dist/webviewis byte-identical. The host bundles grow by 14,222 B each (desktop and web) at commit 3, and by 15 B more at commit 4 (the exports):binaryView.js+11,698 B andbinaryEngine.js+6,221 B, against the provider's −11,372 B;binaryDocument.ts+1,908 B,engineTable.ts+112 B,diskConflict.ts−495 B;P4, through the real editors (#166's runners), on this branch
On commit 3's build (6ee2919). Commit 4 moves the two setRows builders and changes no literal: in the built bundle both still read the same keys in the same order. So P4 was not rerun on it;
npm run verifyand CI's integration suite were. The output is in/tmp/eb-run-v2(MAT and gestures) and/tmp/eb-run-v2b(plain edits)./tmp/eb-run-v0c-main)/tmp/eb-run-p4mat/tmp/eb-run-p4c1521, 63 whole files byte-equal/tmp/eb-run-p4c1521Run 2's first attempt was cut short when every VS Code window on the machine was closed by hand mid-sweep; it was rerun whole on the same build, and these are the rerun's numbers.
Risks, data loss first, and what holds each
engine.beginSave()runs synchronously before the save's first await: it reads, gates and zips (level 6) and takes the edited and unchecked marks. On any throw the host callsnotSaved, which gives them back. On success,savedholds the next save to the text this one wrote, re-baselines, then repaints from the registered tree. saveSettled reads the same EditStackIndex. Held by binarySlddSaveGate, SaveSafety, SaveLanding, KeepsUntouched, SaveOrder (D44) and P4.engine.replacebefore anything changes; the host sets the stamp only after it. A failed revert read falls back torevertToSaved(). Held by binarySlddOutsideChange and the revert cases.engine.backup()is ungated, level 1; the restore opens dirty with no stamp. Held by the backup and restore cases.readyanswered at once. Held by the A/B messages and engineTable.test.ts.For later
createRegisters()and deletes the shared copies, xmlStructuralEdit included.EditStackMirrorname and the forTests seam's uses with them.