Restructure data editor messaging with Message Registry#1647
Restructure data editor messaging with Message Registry#1647stricklandrbls wants to merge 1 commit intoapache:mainfrom
Conversation
65b3fd8 to
8404e0c
Compare
dc8d15a to
55d963f
Compare
#1659 is the issue I created for fixing the CI |
|
@stricklandrbls Will the updates to fix #1659 be added to this PR? My personal input would be, to add those changes here. I would rather have additional changes in this PR and a working CI then to merge this PR and introduce broken CI into main which requires another PR to fix. |
Yeah those changes would fix the CI I created the separate issue because it required multiple additional dev dependencies. I can add that issue as one that will close this PR. |
e5d837d to
d3e7b54
Compare
This PR resolved the CI failing jobs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 50 out of 55 changed files in this pull request and generated 13 comments.
Comments suppressed due to low confidence (1)
src/svelte/src/components/ServerMetrics/ServerMetrics.svelte:31
heartbeatis initialized withomegaEditPort, but later code assigns/usesheartbeat.port(e.g., in theheartbeatlistener and template). This adds a new property at runtime and leaves the declaredomegaEditPortunused/incorrect. Rename the state field to match the payload (port) or keep usingomegaEditPortconsistently.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
72c22db to
ed1adec
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 50 out of 55 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e290c32 to
c59b811
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 54 out of 59 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (1)
src/svelte/src/stores/index.ts:265
- These
derived(...)stores callisRegularSizedFile(), butisRegularSizedFileis backed by a Svelte v5$derivedrune (not a Svelte store). Because it isn’t listed in the dependency array, changes tofileMetricsState.computedSizewon’t trigger recomputation, soeditModecan become stale when file size changes (e.g. on initialcountsmessage). Consider reintroducing a Svelte-storeregularSizedFiledependency, or migrate these derived stores to runes so reactivity stays consistent.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b305f25 to
59f444c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 61 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
59f444c to
93cbaae
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 57 out of 62 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (3)
src/svelte/src/stores/index.ts:263
editModeno longer depends onfileMetricsState.computedSize; it only reruns when the selection changes. If counts/save events flip the file between the special 1-byte case and a regular file, the editor mode stays stale and the UI keeps using the wrong single-vs-multiple-byte path.
src/svelte/src/stores/states.svelte.ts:24formatMsgId()prefixes debug-attached editors withdfdl-, but this check usesincludes()instead ofstartsWith(). Any normal file whose basename happens to containdfdl-will be misclassified as debug-attached and start reacting to debugger byte-position events.
src/svelte/src/stores/index.ts:418applicablehas the same dependency hole aseditModeandoriginalDataSegment: it readsisRegularSizedFile()but never subscribes tofileMetricsState.computedSize. After a size-changing edit/save, the Apply button can stay enabled/disabled for the wrong editing mode until some unrelated store update happens.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| formatMsgId(id: string, isDFDLDebugAttached: boolean = false) { | ||
| let idStr = id.substring(0, UI_MSG_ID_MAX_LEN) | ||
| idStr = idStr.replaceAll(' ', '_') | ||
| return isDFDLDebugAttached ? 'dfdl-' + idStr : idStr | ||
| } |
| .replace('__extension_msg_id__', msgId) | ||
|
|
||
| vsWebview.html = indexHTML | ||
| } |
| ) { | ||
| vscode.window.onDidChangeActiveColorTheme((newTheme) => { |
| export const originalDataSegment = derived( | ||
| [viewport, selectionDataStore, regularSizedFile], | ||
| ([$viewport, $selectionData, $regularSizedFile]) => { | ||
| [viewport, selectionDataStore], | ||
| ([$viewport, $selectionData]) => { | ||
| if (!$viewport.data) return [] | ||
| if (!$regularSizedFile) return $viewport.data | ||
| if (!isRegularSizedFile()) return $viewport.data |
| <Button | ||
| fn={replace} | ||
| description="Replace the current match" | ||
| disabledBy={!replaceable || replaceErrDisplay} | ||
| isDisabled={!replaceable || replaceErrDisplay} | ||
| > |
| addListener('heartbeat', (data) => { | ||
| const { | ||
| latency, | ||
| port, | ||
| serverCpuLoadAverage, | ||
| serverTimestamp, | ||
| serverUptime, | ||
| serverUsedMemory, | ||
| sessionCount, | ||
| serverInfo, | ||
| } = data | ||
| heartbeat.latency = latency | ||
| heartbeat.port = port |
| const languageResponse = await getLanguage( | ||
| this.omegaSessionId, | ||
| startOffset, | ||
| length, | ||
| characterCount!.getByteOrderMark() | ||
| ) | ||
| await this.panel.postMessage('profile', { | ||
| startOffset: startOffset, | ||
| length: length, | ||
| byteProfile: byteProfile, | ||
| numAscii: numAscii(byteProfile), | ||
| language: languageResponse.getLanguage(), | ||
| contentType: contentTypeResponse.getContentType(), | ||
| characterCount: { | ||
| byteOrderMark: characterCount!.getByteOrderMark(), | ||
| byteOrderMarkBytes: characterCount!.getByteOrderMarkBytes(), | ||
| singleByteCount: characterCount!.getSingleByteChars(), | ||
| doubleByteCount: characterCount!.getDoubleByteChars(), | ||
| tripleByteCount: characterCount!.getTripleByteChars(), | ||
| quadByteCount: characterCount!.getQuadByteChars(), | ||
| invalidBytes: characterCount!.getInvalidBytes(), | ||
| }, |
| Most of the open source code in the Saxon product is governed by the Mozilla Public | ||
| License version 2.0, which is reproduced below. | ||
|
|
||
| - 'sax' in extension/dist/ext/extension.js | ||
| - 'sax' in node_modules/vite | ||
| This product bundles 'sax' from the above files. | ||
| These files are available under the BlueOak-1.0.0 license: | ||
|
|
||
| Most of the open source code in the Saxon product is governed by the Blue Oak Model License | ||
| version 1.0.0, which is reproduced below. |
| Most of the open source code in the Saxon product is governed by the Mozilla Public | ||
| License version 2.0, which is reproduced below. | ||
|
|
||
| - 'sax' in extension/dist/ext/extension.js | ||
| - 'sax' in node_modules/vite | ||
| This product bundles 'sax' from the above files. | ||
| These files are available under the BlueOak-1.0.0 license: | ||
|
|
||
| Most of the open source code in the Saxon product is governed by the Blue Oak Model License | ||
| version 1.0.0, which is reproduced below. |
Closes #1659
Closes #1058
Closes #669
Description
Wiki
Review Instructions including Screenshots
Developer Perspective Reviews
this.panel.postMessage(...).Screenshots ( dataEditorClient.ts -> 'fileInfo')
addListener(...).Screenshots ( DataEditorLineFeed.svelte -> 'viewportRefresh')
vscode.postMessage(...).Screenshots ( App.svelte -> 'requestEditedData')
Functionality Review
Verify that all message traffic behaves as expected. Message content can be identified within the ./src/ext_types/message.ts & ./src/ext_types/messageContent.ts