You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "stricklandrbls (via GitHub)" <gi...@apache.org> on 2026/03/26 17:42:59 UTC
[PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
stricklandrbls opened a new pull request, #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647
Closes #[issue]
## Description
- Implemented a _Message Registry_ which provides type-safe message content inspection for messages sent between the data editor, dfdl extension, and the Svelte UI.
- Restructured some legacy Svelte stores into Svelte v5 runes.
- Resolved issues related to message content mismathcing and message content being processed by multiple data editors when the message was intended for a single data editor instance.
## Wiki
- [x] I have determined that no documentation updates are needed for these changes
- [ ] I have added the following documentation for these changes
## Review Instructions including Screenshots
#### Developer Perspective Reviews
1. In any portion of the data editor's extension source, verify that message content is appropriately inspected and the VSCode intellisense engine emits correct types when invoking `this.panel.postMessage(...)`.
<details><summary><b>Screenshots ( dataEditorClient.ts -> 'fileInfo')</b></summary>
<img width="759" height="125" alt="image" src="https://github.com/user-attachments/assets/cbf6e919-8854-4a22-9410-3c8e6de454d6" />
<img width="625" height="72" alt="image" src="https://github.com/user-attachments/assets/b27d68f9-5795-4643-bb3b-a5ace391d88c" />
</details>
2. In any portion of the data editor's Svelte UI source, verify that message content is appropriately inspected and the VSCode intellisense engine emits correct types when invoking `addListener(...)`.
<details><summary><b>Screenshots ( DataEditorLineFeed.svelte -> 'viewportRefresh')</b></summary>
<img width="738" height="242" alt="image" src="https://github.com/user-attachments/assets/db74889f-66e0-4b2a-9432-82b8f1fab6fd" />
</details>
3. In any portion of the data editor's Svelte UI source, verify that message content is appropriately inspected and the VSCode intellisense engine emits correct types when invoking `vscode.postMessage(...)`.
<details><summary><b>Screenshots ( App.svelte -> 'requestEditedData')</b></summary>
<img width="709" height="212" alt="image" src="https://github.com/user-attachments/assets/c7845259-618a-4bcf-b6a9-4cbae2d2bb37" />
</details>
---
#### 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_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3143650390
##########
src/svelte/tsconfig.json:
##########
@@ -16,22 +16,26 @@
*/
{
- "extends": "@tsconfig/svelte/tsconfig.json",
+ "extends": "../../tsconfig.json",
"compilerOptions": {
+ "module": "esnext",
+ "moduleResolution": "bundler",
+ "target": "es2015",
"allowImportingTsExtensions": true,
"noEmit": true,
"inlineSources": true,
- "baseUrl": "./src",
+ "baseUrl": "./",
"paths": {
- "utilities": ["utilities"],
- "layout/*": ["components/layouts/*"],
- "HTMLWrappers/*": ["components/html/*"],
- "editor_components/*": ["components/sections/*"]
+ "utilities/*": ["src/utilities/*"],
+ "layout/*": ["src/components/layouts/*"],
+ "HTMLWrappers/*": ["src/components/html/*"],
+ "editor_components/*": ["src/components/*"],
+ "ext_types": ["../ext_types/index.ts"],
+ "ext_types/*": ["../ext_types/*"],
+ "types": ["../types/*"],
+ "stores/*": ["src/stores/*"]
Review Comment:
`compilerOptions.paths` maps `"types": ["../types/*"]`, but there doesn’t appear to be a `src/types/` directory in the repo. If this is meant to reference generated declaration files, ensure they exist at build/test time; otherwise remove/fix this path mapping to avoid module resolution failures.
##########
src/svelte/vite.config.mjs:
##########
@@ -1,81 +1,59 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
-import { defineConfig } from 'vite'
+// http://www.apache.org/licenses/LICENSE-2.0
+
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { defineConfig, mergeConfig } from 'vitest/config'
import { svelte } from '@sveltejs/vite-plugin-svelte'
-import path from 'path'
+import { fileURLToPath } from 'node:url'
import strip from 'rollup-plugin-strip-code'
-import { fileURLToPath } from 'url'
-import { loadEnvFile } from 'node:process'
import * as fs from 'fs'
+import path from 'path'
+
const r = (p) => fileURLToPath(new URL(p, import.meta.url))
const envFilePath = path.resolve(__dirname, '.env')
+const projectRoot = r('.')
+const distDir = path.resolve(__dirname, '../../', 'dist/views/dataEditor')
+
+const htmlNoncePlugin = () => {
+ return {
+ name: 'nonce',
+ transformIndexHtml(html) {
+ html = html.replace(
+ /<link rel="stylesheet" crossorigin href="\/style.css">/g,
+ `<link rel="stylesheet" crossorigin href="/style.css" nonce="__nonce__"/> `
+ )
+
+ return html.replace(
+ /<script type="module" src="\/src\/main.ts"><\/script>/g,
+ `<script type="module" src="\/index.js" nonce="__nonce__"></script>`
+ )
+ },
+ }
+}
-/** @type {import('vite').UserConfig} */
+/// <reference types="vitest/config"/>
export default defineConfig(({ mode }) => {
if (fs.existsSync(envFilePath)) loadEnvFile(envFilePath)
const debugDataEditor =
process.env.DEBUG_DATAEDITOR == 'on' && mode === 'development'
Review Comment:
`loadEnvFile(envFilePath)` is called here but `loadEnvFile` is not imported or defined anywhere in this module, so evaluating the Vite config will throw a `ReferenceError`. Import `loadEnvFile` from the appropriate package (e.g., `vite`) or replace this with the intended env-loading mechanism (such as `loadEnv`).
##########
src/svelte/tsconfig.json:
##########
@@ -40,7 +44,13 @@
"esModuleInterop": true,
"verbatimModuleSyntax": true
},
- "include": ["src/**/*.ts", "src/**/*.svelte", "src/tests/**/*.test.ts"],
+ "include": [
+ "src/**/.ts",
+ "src/**/*.svelte",
+ "src/**/*.svelte.ts",
Review Comment:
The `include` glob `"src/**/.ts"` looks like a typo and won’t match `*.ts` files (it matches a literal `.ts` path segment). This can cause TypeScript/Svelte typechecking to silently skip most TS sources. Use `"src/**/*.ts"` (and/or add any other intended patterns) instead.
##########
src/dataEditor/ui/svelteWebviewInitializer.ts:
##########
@@ -0,0 +1,282 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as vscode from 'vscode'
+import * as fs from 'fs'
+import { MessageResponseMap, PostMessageArgs } from 'ext_types'
+import { DisplayState } from './displayState'
+
+export interface DataEditorUI extends vscode.Disposable {
+ readonly displayState: DisplayState
+ dispose(): void
+ setTitle(title: string): void
+ getMsgId(): string
+ postMessage<K extends keyof MessageResponseMap>(
+ ...payload: PostMessageArgs<MessageResponseMap, K>
+ ): void
+ reveal: vscode.WebviewPanel['reveal']
+ onDidReceiveMessage(listener: (e: any) => void, thisArg?: any): void
+ onDidDispose(listener: (e: any) => void): void
+}
+class SvelteUIWebviewPanel implements DataEditorUI {
+ static readonly uiViewId: string = 'dataEditor'
+ readonly displayState: DisplayState = new DisplayState()
+
+ constructor(
+ private vscodePanel: vscode.WebviewPanel,
+ private messengerId: string = ''
+ ) {}
+ dispose() {
+ this.vscodePanel.dispose()
+ }
+ getMsgId() {
+ return this.messengerId
+ }
+ setMsgId(id: string) {
+ if (this.messengerId !== '')
+ throw 'Cannot reassign messenger id after initialization.'
+
+ this.messengerId = id
+ }
+ setTitle(title: string) {
+ this.vscodePanel.title = title
+ }
+
+ postMessage<K extends keyof MessageResponseMap>(
+ ...msg: PostMessageArgs<MessageResponseMap, K>
+ ) {
+ const [type, payload] = msg as [K, MessageResponseMap[K]]
+ this.vscodePanel.webview.postMessage({
+ command: type,
+ id: this.messengerId,
+ data: payload,
+ })
+ }
+
+ public onDidReceiveMessage(listener: (e: any) => void, thisArg?: any) {
+ this.vscodePanel.webview.onDidReceiveMessage(listener, thisArg)
+ }
+
+ public readonly reveal = () => {
+ this.vscodePanel.reveal()
+ }
+
+ public onDidDispose(listener: (e: any) => void) {
+ this.vscodePanel.onDidDispose(listener)
+ }
+}
+
+export type SveltePanelArgs = {
+ title: string
+ column: vscode.ViewColumn
+ uiMsgId: string
+}
+
+interface SvelteInitializerState {
+ isContextValid: boolean
+ getExtensionCtx(): vscode.ExtensionContext
+ getSvelteWebviewInitializer(): SvelteWebviewInitializer
+}
+const InvalidInitializerState: SvelteInitializerState = {
+ isContextValid: false,
+ getSvelteWebviewInitializer: () => {
+ throw 'SvelteWebviewInitializer is in an invalid state'
+ },
+ getExtensionCtx: function (): vscode.ExtensionContext {
+ throw 'SvelteWebviewInitializer is in an invalid state'
+ },
+}
+
+const ValidInitializerState: SvelteInitializerState = {
+ isContextValid: true,
+ getSvelteWebviewInitializer: () => {
+ return SvelteWebviewMgr
+ },
+ getExtensionCtx: () => {
+ return ExtensionContextRef
+ },
+}
+
+const UI_MSG_ID_MAX_LEN = 32
+
+class SvelteWebviewInitializer {
+ static state: SvelteInitializerState = InvalidInitializerState
+ constructor() {}
+ formatMsgId(id: string, isDFDLDebugAttached: boolean = false) {
+ let idStr = id.substring(0, UI_MSG_ID_MAX_LEN)
+ idStr = idStr.replaceAll(' ', '_')
+ return isDFDLDebugAttached ? 'dfdl-' + idStr : idStr
+ }
+ createSveltePanel(args: SveltePanelArgs): SvelteUIWebviewPanel {
+ const opts = this.getWebviewOptions()
+ const ret = vscode.window.createWebviewPanel(
+ SvelteUIWebviewPanel.uiViewId,
+ args.title,
+ args.column,
+ {
+ ...opts,
+ }
+ )
+ this.setHtmlContent(ret, args.uiMsgId)
+
+ return new SvelteUIWebviewPanel(ret, args.uiMsgId)
+ }
+
+ // get the HTML content for the webview
+ private setHtmlContent(vsPanel: vscode.WebviewPanel, msgId: string) {
+ let vsWebview = vsPanel.webview
+ const nonce = this.getNonce()
+
+ const scriptUri = this.getResourceUri('js', ExtensionContextRef, (uri) => {
+ return vsWebview.asWebviewUri(uri)
+ })
+ const stylesUri = this.getResourceUri('css', ExtensionContextRef, (uri) => {
+ return vsWebview.asWebviewUri(uri)
+ })
+
+ let indexHTML = this.injectNonce(
+ this.getIndexHTML(ExtensionContextRef),
+ vsWebview,
+ nonce,
+ scriptUri
+ )!
+ indexHTML = indexHTML
+ .replace(/src="\.\/index.js"/, `src="${scriptUri.toString()}"`)
+ .replace(/href="\.\/style.css"/, `href="${stylesUri.toString()}"`)
+ .replaceAll(/nonce="__nonce__"/g, `nonce="${nonce}"`)
+ .replace('__extension_msg_id__', msgId)
+
+ vsWebview.html = indexHTML
+ }
+ private injectNonce(
+ html: string,
+ webView: vscode.Webview,
+ nonce: string,
+ scriptsUri: vscode.Uri
+ ) {
+ let ret = html.replaceAll(
+ '<head>',
+ `<head><meta http-equiv="Content-Security-Policy" content="default-src ${webView.cspSource}; font-src ${webView.cspSource}; style-src 'self' 'unsafe-inline' ${webView.cspSource}; img-src ${webView.cspSource}; script-src 'nonce-${nonce}' ${webView.cspSource};">`
+ )
+ return ret
+ }
+ private getIndexHTML(context: vscode.ExtensionContext) {
+ const indexFile = vscode.Uri.joinPath(
+ context.extensionUri,
+ 'dist',
+ 'views',
+ 'dataEditor',
+ 'index.html'
+ )
+ const indexContent = fs.readFileSync(indexFile.fsPath).toString()
+ return indexContent
+ }
+ // get a nonce for use in a content security policy
+ private getNonce(): string {
+ let text = ''
+ const possible =
+ 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'
+ for (let i = 0; i < 32; i++) {
+ text += possible.charAt(Math.floor(Math.random() * possible.length))
+ }
+ return text
+ }
+ private getWebviewOptions() {
+ const opts: vscode.WebviewPanelOptions & vscode.WebviewOptions = {
+ enableScripts: true,
+ retainContextWhenHidden: true,
+ localResourceRoots: [
+ this.getSvelteAppDistributionFolderUri(ExtensionContextRef),
+ this.getSvelteAppDistributionViewFolderUri(
+ ExtensionContextRef,
+ SvelteUIWebviewPanel.uiViewId
+ ),
+ ],
+ }
+ return opts
+ }
+
+ // get the svelte app distribution folder uri
+ private getSvelteAppDistributionFolderUri(
+ context: vscode.ExtensionContext
+ ): vscode.Uri {
+ return vscode.Uri.joinPath(context.extensionUri, 'dist')
+ }
+
+ // get the svelte app distribution view folder uri
+ private getSvelteAppDistributionViewFolderUri(
+ context: vscode.ExtensionContext,
+ view: string
+ ): vscode.Uri {
+ return vscode.Uri.joinPath(context.extensionUri, 'dist', 'views', view)
+ }
+
+ private getResourceUri(
+ item: 'index' | 'css' | 'js',
+ context: vscode.ExtensionContext,
+ uriDecorator?: (uriPath: vscode.Uri) => any
+ ): vscode.Uri
+
+ private getResourceUri<R>(
+ item: 'index' | 'css' | 'js',
+ context: vscode.ExtensionContext,
+ uriDecorator: (uriPath: vscode.Uri) => R
+ ): R
+ private getResourceUri<R>(
+ item: 'index' | 'css' | 'js',
+ context: vscode.ExtensionContext,
+ uriDecorator?: (uriPath: vscode.Uri) => R
+ ): vscode.Uri | R {
+ let resourceFile = ''
+ switch (item) {
+ case 'index':
+ resourceFile = item + '.html'
+ break
+ case 'css':
+ resourceFile = 'style.css'
+ break
+ case 'js':
+ resourceFile = 'index.js'
+ break
+ }
+ let ret = vscode.Uri.joinPath(
+ context.extensionUri,
+ 'dist',
+ 'views',
+ 'dataEditor',
+ resourceFile
+ )
+ return uriDecorator ? uriDecorator(ret) : ret
+ }
+}
+
+const SvelteWebviewMgr: SvelteWebviewInitializer =
+ new SvelteWebviewInitializer()
+let ExtensionContextRef: vscode.ExtensionContext
+
+export function getSvelteWebviewInitializer() {
+ return SvelteWebviewInitializer.state['getSvelteWebviewInitializer']()
+}
+
+export function startSvelteWebviewInitializer(ctx: vscode.ExtensionContext) {
+ if (SvelteWebviewInitializer.state === ValidInitializerState) {
+ console.error('The SvelteWebviewInitializer is already active')
Review Comment:
`startSvelteWebviewInitializer()` logs `console.error` when already initialized, but `DataEditorClient.open()` calls it on every open. This will produce noisy error logs during normal use with multiple data editors. Consider making this idempotent without logging an error (or downgrade to `console.debug`/no-op) when the initializer is already active.
```suggestion
```
##########
src/svelte/tests/stores/fileMetricsState.svelte.test.ts:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { describe, it, expect } from 'vitest'
+import {
+ canRevert,
+ canUndo,
+ fileMetricsState,
+ isRegularSizedFile,
+ saveable,
+} from '../../src/components/Header/fieldsets/FileMetrics.svelte.ts'
+
+describe('FileMetricsState Runes', () => {
+ describe('regularSizedFile', () => {
+ it('should report boolean value derived from computedSize', () => {
+ ;[
+ { size: 1024, expect: true },
+ { size: 1, expect: false },
+ ].forEach((testValues) => {
+ fileMetricsState.computedSize = testValues.size
+ expect(isRegularSizedFile()).toBe(testValues.expect)
+ })
+ })
+ })
+ describe('saveable', () => {
+ it('should report boolean value derived from changeCount', () => {
+ ;[
+ { count: 2, expect: true },
+ { count: 0, expect: false },
+ ].forEach((testValues) => {
+ fileMetricsState.changeCount = testValues.count
+ expect(saveable()).toBe(testValues.expect)
+ })
+ })
+ })
+ describe('canUndo', () => {
+ it('should report boolean value derived from changeCount', () => {
+ ;[
+ { count: 2, expect: true },
+ { count: 0, expect: false },
+ ].forEach((testValues) => {
+ fileMetricsState.changeCount = testValues.count
+ expect(canUndo()).toBe(testValues.expect)
+ })
+ })
+ })
+ describe('canRedo', () => {
+ it('should report boolean value derived from undoCount', () => {
+ ;[
+ { count: 2, expect: true },
+ { count: 0, expect: false },
+ ].forEach((testValues) => {
+ fileMetricsState.changeCount = testValues.count
+ expect(canUndo()).toBe(testValues.expect)
+ })
Review Comment:
The `canRedo` test is exercising the wrong state and selector: it sets `fileMetricsState.changeCount` and asserts on `canUndo()`, even though the describe block says it should be derived from `undoCount`. Update this test to set `fileMetricsState.undoCount` and assert `canRedo()` so it validates the intended behavior.
##########
src/dataEditor/include/utils.ts:
##########
@@ -16,9 +16,16 @@
*/
import { debug } from 'vscode'
-export function isDFDLDebugSessionActive(): boolean {
- return (
- debug.activeDebugSession !== undefined &&
- debug.activeDebugSession.type === 'dfdl'
- )
+export function isDFDLDebugSessionActive(file: string): boolean {
+ if (debug.activeDebugSession === undefined) return false
+ if (debug.activeDebugSession!.type !== 'dfdl') return false
+ if (debug.activeDebugSession!.configuration.data !== file) return false
+ return true
+}
+
+export function toEncoding(encoding: string): BufferEncoding {
+ if (!Buffer.isEncoding(encoding)) {
+ console.error(`Value (${encoding}) is not a valid BufferEncoding type`)
Review Comment:
`toEncoding()` logs when the encoding is invalid, but still returns the invalid value cast as `BufferEncoding`. Callers then pass it into `Buffer.from(..., encoding)` / `.toString(encoding)`, which can throw at runtime for unknown encodings. Consider throwing on invalid input or returning a safe fallback/default (or `undefined`) and handling that explicitly at call sites.
```suggestion
console.error(`Value (${encoding}) is not a valid BufferEncoding type`)
return 'utf8'
```
##########
src/svelte/src/App.svelte:
##########
@@ -64,21 +56,28 @@ limitations under the License.
} from './components/DataDisplays/CustomByteDisplay/BinaryData'
import { byte_count_divisible_offset } from './utilities/display'
import Help from './components/layouts/Help.svelte'
- import { viewportByteIndicators } from 'utilities/highlights'
+ import { EditByteModes, type BytesPerRow } from 'ext_types'
+ import { VIEWPORT_SCROLL_INCREMENT } from './stores/configuration'
+ import { getUIMsgId, setUIMsgId } from './stores/states.svelte'
+ import { vscode } from './utilities/vscode'
+ import { isRegularSizedFile } from './components/Header/fieldsets/FileMetrics.svelte.ts'
+ import { viewportByteIndicators } from 'utilities/highlights.ts'
+ setUIMsgId(
+ document.getElementById('app')?.attributes['extension_msg_id'].value
+ )
+
+ const { postMessage, addListener } = vscode.getMessenger(getUIMsgId())
Review Comment:
`setUIMsgId(...)` is passed a value that can be `undefined` (if `#app` is missing or the attribute isn’t present), but downstream code assumes a valid non-empty ID when calling `vscode.getMessenger(getUIMsgId())`. Add a guard/fallback (e.g., throw with a clear error, or default to a generated ID for dev-server usage when the placeholder `__extension_msg_id__` is present).
```suggestion
function resolveUIMsgId(): string {
const appElement = document.getElementById('app')
const msgId = appElement?.getAttribute('extension_msg_id')?.trim()
if (msgId && msgId !== '__extension_msg_id__') {
return msgId
}
if (msgId === '__extension_msg_id__') {
const generatedMsgId = `dev-ui-msg-${Date.now()}-${Math.random()
.toString(36)
.slice(2)}`
console.warn(
'Using generated UI message ID because the dev placeholder "__extension_msg_id__" was not replaced.'
)
return generatedMsgId
}
throw new Error(
'Missing required "extension_msg_id" attribute on #app; cannot initialize VS Code messenger.'
)
}
const uiMsgId = resolveUIMsgId()
setUIMsgId(uiMsgId)
const { postMessage, addListener } = vscode.getMessenger(uiMsgId)
```
##########
src/dataEditor/ui/svelteWebviewInitializer.ts:
##########
@@ -0,0 +1,282 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as vscode from 'vscode'
+import * as fs from 'fs'
+import { MessageResponseMap, PostMessageArgs } from 'ext_types'
+import { DisplayState } from './displayState'
+
+export interface DataEditorUI extends vscode.Disposable {
+ readonly displayState: DisplayState
+ dispose(): void
+ setTitle(title: string): void
+ getMsgId(): string
+ postMessage<K extends keyof MessageResponseMap>(
+ ...payload: PostMessageArgs<MessageResponseMap, K>
+ ): void
+ reveal: vscode.WebviewPanel['reveal']
+ onDidReceiveMessage(listener: (e: any) => void, thisArg?: any): void
+ onDidDispose(listener: (e: any) => void): void
+}
+class SvelteUIWebviewPanel implements DataEditorUI {
+ static readonly uiViewId: string = 'dataEditor'
+ readonly displayState: DisplayState = new DisplayState()
+
+ constructor(
+ private vscodePanel: vscode.WebviewPanel,
+ private messengerId: string = ''
+ ) {}
+ dispose() {
+ this.vscodePanel.dispose()
+ }
+ getMsgId() {
+ return this.messengerId
+ }
+ setMsgId(id: string) {
+ if (this.messengerId !== '')
+ throw 'Cannot reassign messenger id after initialization.'
Review Comment:
`setMsgId` throws a string literal. Throwing `Error` instances preserves stack traces and is easier to handle consistently. Replace `throw '...'` with `throw new Error('...')`.
```suggestion
throw new Error('Cannot reassign messenger id after initialization.')
```
##########
src/dataEditor/dataEditorClient.ts:
##########
@@ -184,50 +187,56 @@ export class DataEditorClient implements vscode.Disposable {
this.disposables[i].dispose()
}
- show(): void {
+ public show(): void {
this.panel.reveal()
}
+
public static async open(
context: vscode.ExtensionContext,
- view: string,
configVars: editor_config.IConfig,
fileToEdit: string = ''
): Promise<DataEditorClient | undefined> {
- const title = !fileToEdit ? 'Data Editor' : path.basename(fileToEdit)
+ startSvelteWebviewInitializer(context)
+ const title = !fileToEdit
+ ? 'Data Editor' + (OPEN_EDITORS.size + 1).toString()
+ : path.basename(fileToEdit)
const column =
- fileToEdit !== '' ? vscode.ViewColumn.Two : vscode.ViewColumn.Active
-
- const panel = vscode.window.createWebviewPanel(view, title, column, {
- enableScripts: true,
- retainContextWhenHidden: true,
- })
-
- const editor = new DataEditorClient(
- context,
- view,
+ fileToEdit === '' ? vscode.ViewColumn.Two : vscode.ViewColumn.Active
Review Comment:
The `column` selection appears inverted: when `fileToEdit` is non-empty (the common case), this now opens the webview in `ViewColumn.Active`, whereas previously files opened in `ViewColumn.Two`. If the intent is to open the data editor beside the current editor (and not replace it), swap this conditional back to `fileToEdit !== '' ? ViewColumn.Two : ViewColumn.Active`.
```suggestion
fileToEdit !== '' ? vscode.ViewColumn.Two : vscode.ViewColumn.Active
```
##########
src/ext_types/messageContent.ts:
##########
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import {
+ ApplyChangesRequest,
+ ChangesInfoResponse,
+ CountResponse,
+ DFDLDataBytePos,
+ EditedDataRequest,
+ EditedDataResponse,
+ EditorOnChangeRequest,
+ EditorOnChangeResponse,
+ FileInfoResponse,
+ HeartbeatResponse,
+ NotificationRequest,
+ ProfileRequest,
+ ProfileResponse,
+ ReplaceRequest,
+ ReplaceResponse,
+ SaveAsResponse,
+ SaveResponse,
+ SaveSegmentRequest,
+ ScrollViewportRequest,
+ SearchRequest,
+ SearchResponse,
+ SetUIThemeResponse,
+ ViewportRefreshResponse,
+} from './messages'
+import { EditorRequestIds, isEditorMessageId } from './messageIds'
+
+type MessageCommands =
+ | 'clearChanges'
+ | 'applyChanges'
+ | 'editorOnChange'
+ | 'fileInfo'
+ | 'heartbeat'
+ | 'profile'
+ | 'redoChange'
+ | 'replaceResults'
+ | 'requestEditedData'
+ | 'save'
+ | 'saveAs'
+ | 'saveSegment'
+ | 'scrollViewport'
+ | 'search'
+ | 'replace'
+ | 'setUITheme'
+ | 'showMessage'
+ | 'undoChange'
+ | 'viewportRefresh'
+
+type CommandMap = {
+ [K in MessageCommands]: any
+}
+export interface MessageRequestMap extends CommandMap {
+ counts: never
+ clearChanges: never
+ applyChanges: ApplyChangesRequest
+ editorOnChange: EditorOnChangeRequest
+ fileInfo: never
+ heartbeat: never
+ profile: ProfileRequest
+ redoChange: never
+ replaceResults: never
+ requestEditedData: EditedDataRequest
+ save: never
+ saveAs: never
+ saveSegment: SaveSegmentRequest
+ scrollViewport: ScrollViewportRequest
+ search: SearchRequest
+ replace: ReplaceRequest
+ undoChange: never
+ viewportRefresh: never
+ showMessage: NotificationRequest
+}
+
+export interface MessageResponseMap extends CommandMap {
+ clearChanges: void
+ applyChanges: ChangesInfoResponse
+ editorOnChange: EditorOnChangeResponse
+ fileInfo: FileInfoResponse
+ counts: CountResponse
+ profile: ProfileResponse
+ redoChange: void
+ replaceResults: ReplaceResponse
+ requestEditedData: EditedDataResponse
+ save: SaveResponse
+ saveAs: SaveAsResponse
+ saveSegment: void
+ scrollViewport: void
+ search: SearchResponse
+ replace: ReplaceResponse
+ undoChange: void
+ viewportRefresh: ViewportRefreshResponse
+ showMessage: void
+ setUITheme: SetUIThemeResponse
+ heartbeat: HeartbeatResponse
+ bytePos1b: DFDLDataBytePos
+}
+
+export type ExtensionMessageKeys =
+ | 'showMessage'
+ | 'setUITheme'
+ | 'editorOnChange'
+
+export type DataEditorMessageKeys = Exclude<
+ MessageCommands,
+ ExtensionMessageKeys
+>
+export type DataEditorMessageRequests = Pick<
+ MessageRequestMap,
+ DataEditorMessageKeys
+>
+export type DataEditorMessageResponses = Pick<
+ MessageResponseMap,
+ DataEditorMessageKeys
+>
+/**
+ * Key indexable interface to templated type inference of available messages sent between
+ * the components of the DFDL VSCode extension.
+ */
+export type ExtensionMessageRequests = Pick<
+ MessageRequestMap,
+ ExtensionMessageKeys
+>
+
+export interface DFDLSessionMessageResponses extends MessageResponseMap {
+ bytePos1b: DFDLDataBytePos
+}
+
+export type VSMessagePackage = {
+ uiId: string
Review Comment:
The `VSMessagePackage` type uses a `uiId` property, but the webview-side code constructs messages with an `id` property (`{ id, payload }`). This mismatch defeats the goal of routing messages to a single editor instance and makes the typing misleading. Align the shared type with the actual message shape (rename `uiId` to `id`, or update the sender/receiver to consistently use `uiId`).
```suggestion
id: string
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4314278721
> The only requirement is that if it's a runtime dependency we must mention sax-js and the blue oak license in build/package/LICENSE. And we probably need to update build/license_data.json to include the blue oak license as an allowed license so CI passes
The `sax` package is a transitive dependency from one of our current _development_ dependencies (`vite@8.0.0`). However, I did notice that there is already a `sax-js` entry in the license file but is states the license is **ISC**. Is this correct?
https://github.com/ctc-oss/daffodil-vscode/blob/8ec61df4861682d04d165687d105b2771d2d9262/build/package/LICENSE#L4977-L4981
---
I can go ahead and make those changes to the license data file for BlueOak licenses.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3141594375
##########
src/ext_types/messageContent.ts:
##########
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import {
+ ApplyChangesRequest,
+ ChangesInfoResponse,
+ CountResponse,
+ DFDLDataBytePos,
+ EditedDataRequest,
+ EditedDataResponse,
+ EditorOnChangeRequest,
+ EditorOnChangeResponse,
+ FileInfoResponse,
+ HeartbeatResponse,
+ NotificationRequest,
+ ProfileRequest,
+ ProfileResponse,
+ ReplaceRequest,
+ ReplaceResponse,
+ SaveAsResponse,
+ SaveResponse,
+ SaveSegmentRequest,
+ ScrollViewportRequest,
+ SearchRequest,
+ SearchResponse,
+ SetUIThemeResponse,
+ ViewportRefreshResponse,
+} from './messages'
+import { EditorRequestIds, isEditorMessageId } from './messageIds'
+
+type MessageCommands =
+ | 'clearChanges'
+ | 'applyChanges'
+ | 'editorOnChange'
+ | 'fileInfo'
+ | 'heartbeat'
+ | 'profile'
+ | 'redoChange'
+ | 'replaceResults'
+ | 'requestEditedData'
+ | 'save'
+ | 'saveAs'
+ | 'saveSegment'
+ | 'scrollViewport'
+ | 'search'
+ | 'replace'
+ | 'setUITheme'
+ | 'showMessage'
+ | 'undoChange'
+ | 'viewportRefresh'
+
+type CommandMap = {
+ [K in MessageCommands]: any
+}
+export interface MessageRequestMap extends CommandMap {
+ counts: never
+ clearChanges: never
+ applyChanges: ApplyChangesRequest
+ editorOnChange: EditorOnChangeRequest
+ fileInfo: never
+ heartbeat: never
+ profile: ProfileRequest
+ redoChange: never
+ replaceResults: never
+ requestEditedData: EditedDataRequest
+ save: never
+ saveAs: never
+ saveSegment: SaveSegmentRequest
+ scrollViewport: ScrollViewportRequest
+ search: SearchRequest
+ replace: ReplaceRequest
+ undoChange: never
+ viewportRefresh: never
+ showMessage: NotificationRequest
+}
+
+export interface MessageResponseMap extends CommandMap {
+ clearChanges: void
+ applyChanges: ChangesInfoResponse
+ editorOnChange: EditorOnChangeResponse
+ fileInfo: FileInfoResponse
+ counts: CountResponse
+ profile: ProfileResponse
+ redoChange: void
+ replaceResults: ReplaceResponse
+ requestEditedData: EditedDataResponse
Review Comment:
Type checking in this file is reporting no errors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3237991776
##########
src/svelte/src/components/Header/fieldsets/SearchReplace.svelte:
##########
@@ -403,7 +388,7 @@ limitations under the License.
<Button
fn={replace}
description="Replace the current match"
- disabledBy={!replaceable || replaceErrDisplay}
+ isDisabled={!replaceable || replaceErrDisplay}
>
Review Comment:
I don't think this is the case. I haven't seen the replace button stuck in am enabled state. I will test it out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3141635275
##########
src/svelte/src/components/Header/fieldsets/FileMetrics.svelte:
##########
@@ -17,38 +17,39 @@ limitations under the License.
<script lang="ts">
import Button from '../../Inputs/Buttons/Button.svelte'
import FlexContainer from '../../layouts/FlexContainer.svelte'
- import { MessageCommand } from '../../../utilities/message'
- import { vscode } from '../../../utilities/vscode'
- import { saveable, fileMetrics, replaceQuery } from '../../../stores'
import { createEventDispatcher } from 'svelte'
import SidePanel from '../../layouts/SidePanel.svelte'
import ByteFrequencyGraph from '../../DataMetrics/DataMetrics.svelte'
- import { viewport } from '../../../stores'
import { humanReadableByteLength } from '../../../utilities/display'
import { DATA_PROFILE_MAX_LENGTH } from '../../../stores/configuration'
import Tooltip from '../../layouts/Tooltip.svelte'
import ISO6391 from 'iso-639-1'
+ import { getUIMsgId } from 'stores/states.svelte'
+ import { vscode } from 'utilities/vscode'
+ import {
+ canRedo,
+ canRevert,
+ canUndo,
+ fileMetricsState,
+ saveable,
+ } from './FileMetrics.svelte.ts'
+
const eventDispatcher = createEventDispatcher()
+ const { postMessage, addListener } = vscode.getMessenger(getUIMsgId())
+
let displayOpts = false
let isProfilerOpen = false
- let canUndo: boolean
- let canRedo: boolean
- let canRevert: boolean
let startOffset: number = 0
let length: number = 0
Review Comment:
outdated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4314517592
> > The only requirement is that if it's a runtime dependency we must mention sax-js and the blue oak license in build/package/LICENSE. And we probably need to update build/license_data.json to include the blue oak license as an allowed license so CI passes
>
> The `sax` package is a transitive dependency from one of our current _development_ dependencies (`vite@8.0.0`). However, I did notice that there is already a `sax-js` entry in the license file but is states the license is **ISC**. Is this correct?
>
> **`build/package/LICENSE::4978`**
>
> ```
> - 'sax-js' in extension/dist/ext/extension.js
> This produce bundles 'sax-js' from the above files
> These files are available under the ISC License:
> ```
>
> I can go ahead and make those changes to the license data file for BlueOak licenses.
So I would check if there are two different versions of sax-js in the dependencies. If there is one or more and all are >= 1.4.2 then overwrite the license data in that entry with the BlueOak. But if there is one or more instances of sax-js <= 1.4.1 keep that entry and add a second entry for sax-js using the BlueOak license. I would specify `'sax-js' in extension/dist/ext/extension.js` and `'sax-js' >= 1.4.2 in extension/dist/ext/extension.js` for the entries. Would this be the correct approach @stevedlawrence ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4301819795
It looks as if the failing CI jobs are for transitive dependencies of `vite@8.0.3` that contain unapproved licenses.
I will have to investigate how to resolve this. @shanedell @scholarsmate @hdalsania have either of you run into this kind of issue before when updating packages?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4313792846
> @stevedlawrence What would we need to be done about the `sax` package? It uses the BlueOak-1.0.0 LICENSE, I am not finding anywhere documenting if that is not allowed or allowed so I wanted to get your input.
Blue Oak Model License 1.0.0 is marked as a category A license on this page: https://www.apache.org/legal/resolved.html. So we are free to use it with out any real restrictions. The only requirement is that if it's a runtime dependency we must mention sax-js and the blue oak license in `build/package/LICENSE`. And we probably need to update `build/license_data.json` to include the blue oak license as an allowed license so CI passes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3141636303
##########
src/svelte/vite.config.mjs:
##########
@@ -1,81 +1,59 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
-import { defineConfig } from 'vite'
+// http://www.apache.org/licenses/LICENSE-2.0
+
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { defineConfig, mergeConfig } from 'vitest/config'
import { svelte } from '@sveltejs/vite-plugin-svelte'
-import path from 'path'
+import { fileURLToPath } from 'node:url'
import strip from 'rollup-plugin-strip-code'
-import { fileURLToPath } from 'url'
-import { loadEnvFile } from 'node:process'
import * as fs from 'fs'
+import path from 'path'
+
Review Comment:
loadEnvFile is a `vite` native function and there are no exceptions thrown when it runs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3161093211
##########
src/svelte/src/components/ServerMetrics/ServerMetrics.svelte:
##########
@@ -15,8 +15,10 @@ See the License for the specific language governing permissions and
limitations under the License.
-->
<script lang="ts">
- import { MessageCommand } from '../../utilities/message'
+ import { getUIMsgId } from 'stores/states.svelte'
import FlexContainer from '../layouts/FlexContainer.svelte'
+ import { vscode } from 'utilities/vscode'
+ const { addListener } = vscode.getMessenger(getUIMsgId())
Review Comment:
`vscode.getMessenger(getUIMsgId())` is evaluated at module init time, before `setUIMsgId(...)` runs in `App.svelte`. That means this listener will likely be registered with an empty/placeholder id and will never receive events once the real id is set (because `_addListener` filters by id). Defer creating the messenger until the msg id is initialized (e.g. in `onMount` / a reactive statement) or make the messenger read the current id at dispatch time.
##########
src/dataEditor/include/server/heartbeat/index.ts:
##########
@@ -28,10 +28,15 @@ export function updateHeartbeatInterval(activeSessions: string[]) {
getHeartbeatIntervalId =
activeSessions.length > 0
? setInterval(async () => {
- heartbeatInfo = await getServerHeartbeat(
+ const heartbeat = await getServerHeartbeat(
activeSessions,
HEARTBEAT_INTERVAL_MS * activeSessions.length
)
+ const info = await getServerInfo()
+ heartbeatInfo.serverHeartbeat = {
+ ...heartbeat,
+ serverInfo: { ...info },
+ }
})
Review Comment:
`setInterval` is missing the interval duration argument here. As written, Node will treat the delay as 0ms/undefined and the server heartbeat polling will run in a tight loop, which can peg CPU and overload the Ωedit server. Pass an explicit delay (likely `HEARTBEAT_INTERVAL_MS * activeSessions.length`) as the second argument to `setInterval`.
##########
src/svelte/src/utilities/messages.ts:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import { type MessageResponseMap } from 'ext_types'
+import {
+ isEditorMessageId,
+ isEditorResponseId,
+} from '../../../ext_types/messageIds'
+
+export type IncomingMessage = {
+ command: keyof MessageResponseMap
+ id: string
+ data: MessageResponseMap[keyof MessageResponseMap]
+}
+
+function isEditorMessage(msg: any): msg is IncomingMessage {
+ return msg && isEditorMessageId(msg.command)
+}
+function isEditorResponse(msg: any): msg is IncomingMessage {
+ return msg && isEditorResponseId(msg.command)
+}
Review Comment:
`isEditorMessage` is declared but never used. With `noUnusedLocals` enabled in the TS config, this will fail typechecking/build. Remove it or use it (e.g. to validate/handle incoming request messages if intended).
##########
src/svelte/src/components/DataMetrics/DataMetrics.svelte:
##########
@@ -16,15 +16,18 @@ limitations under the License.
-->
<script lang="ts">
import Button from '../Inputs/Buttons/Button.svelte'
- import { vscode } from '../../utilities/vscode'
- import { MessageCommand } from '../../utilities/message'
import { onMount } from 'svelte'
import Input from '../Inputs/Input/Input.svelte'
import { addressRadix, viewport } from '../../stores'
import { DATA_PROFILE_MAX_LENGTH } from '../../stores/configuration'
import { radixToString, regexEditDataTest } from '../../utilities/display'
import ISO6391 from 'iso-639-1'
import Tooltip from '../layouts/Tooltip.svelte'
+ import { getUIMsgId } from 'stores/states.svelte'
+ import { vscode } from 'utilities/vscode'
+ import { fileMetricsState } from 'editor_components/Header/fieldsets/FileMetrics.svelte.ts'
Review Comment:
`fileMetricsState` is imported here but never referenced in the component. With `noUnusedLocals` enabled, this unused import will break the typecheck/build. Remove the import or use it (e.g. to clamp `startOffset/length` against current file size if that was the intention).
```suggestion
```
##########
src/svelte/src/components/Header/fieldsets/FileMetrics.svelte:
##########
@@ -17,38 +17,39 @@ limitations under the License.
<script lang="ts">
import Button from '../../Inputs/Buttons/Button.svelte'
import FlexContainer from '../../layouts/FlexContainer.svelte'
- import { MessageCommand } from '../../../utilities/message'
- import { vscode } from '../../../utilities/vscode'
- import { saveable, fileMetrics, replaceQuery } from '../../../stores'
import { createEventDispatcher } from 'svelte'
import SidePanel from '../../layouts/SidePanel.svelte'
import ByteFrequencyGraph from '../../DataMetrics/DataMetrics.svelte'
- import { viewport } from '../../../stores'
import { humanReadableByteLength } from '../../../utilities/display'
import { DATA_PROFILE_MAX_LENGTH } from '../../../stores/configuration'
import Tooltip from '../../layouts/Tooltip.svelte'
import ISO6391 from 'iso-639-1'
+ import { getUIMsgId } from 'stores/states.svelte'
+ import { vscode } from 'utilities/vscode'
+ import {
+ canRedo,
+ canRevert,
+ canUndo,
+ fileMetricsState,
+ saveable,
+ } from './FileMetrics.svelte.ts'
+
const eventDispatcher = createEventDispatcher()
+ const { postMessage, addListener } = vscode.getMessenger(getUIMsgId())
+
Review Comment:
`vscode.getMessenger(getUIMsgId())` is called at module init time, so it will likely capture `uiMsgId === ''` (since `setUIMsgId` runs later in `App.svelte`). As a result these `addListener(...)` handlers may never fire because the dispatched events are filtered by id. Create the messenger after the id is known (e.g. in `onMount`) or refactor the messenger to fetch the current id when handling events.
##########
src/svelte/src/components/DataDisplays/CustomByteDisplay/SelectedByteEdit.svelte:
##########
@@ -333,7 +335,6 @@ limitations under the License.
class:inProgress
style:width={elementDivWidth}
Review Comment:
In the `EditActionRestrictions.None` branch the `<input>` no longer binds to `$editorSelection`, but the validity/in-progress logic above is based on `$editorSelection.length`, and the parent `handleEditorEvent` path relies on `$editorSelection` to request encoded data. Without updating the store as the user types, the UI state and outgoing messages will be based on stale data. Bind the input value back to the store (or update the store in `on:input`) so downstream logic stays consistent.
```suggestion
style:width={elementDivWidth}
bind:value={$editorSelection}
```
##########
src/dataEditor/dataEditorClient.ts:
##########
@@ -625,108 +530,227 @@ export class DataEditorClient implements vscode.Disposable {
}
break
- case MessageCommand.saveSegment:
+ case 'applyChanges':
{
- const uri = await vscode.window.showSaveDialog({
- title: 'Save Segment',
- saveLabel: 'Save',
- })
- if (uri && uri.fsPath) {
- await this.saveFileSegment(
- uri.fsPath,
- message.data.offset,
- message.data.length
- )
+ const { offset, edited_segment, original_segment } =
+ getRequestPayloadType<typeof command>(incomingMessage.payload[1])
+ await edit(
+ this.omegaSessionId,
+ offset,
+ original_segment,
+ edited_segment
+ )
+ await this.sendChangesInfo()
+ }
+ break
+ case 'undoChange':
+ {
+ await undo(this.omegaSessionId)
+ await this.sendChangesInfo()
+ this.panel.postMessage('clearChanges')
+ }
+ break
+ case 'redoChange':
+ {
+ await redo(this.omegaSessionId)
+ await this.sendChangesInfo()
+ this.panel.postMessage('clearChanges')
+ }
+ break
+ case 'clearChanges':
+ {
+ if (
+ (await vscode.window.showInformationMessage(
+ 'Are you sure you want to revert all changes?',
+ { modal: true },
+ 'Yes',
+ 'No'
+ )) === 'Yes'
+ ) {
+ await clear(this.omegaSessionId)
+ await this.sendChangesInfo()
+ this.panel.postMessage('clearChanges')
}
}
break
-
- case MessageCommand.requestEditedData:
+ case 'requestEditedData':
{
- const [selectionData, selectionDisplay] = fillRequestData(message)
+ const payload = getRequestPayloadType<typeof command>(
+ incomingMessage.payload[1]
+ )
+ // const {} = incomingMessage.payload as MessageRequestMap['']
+ const [selectionData, selectionDisplay] = fillRequestData(payload)
- await this.panel.webview.postMessage({
- command: MessageCommand.requestEditedData,
- data: {
- data: Uint8Array.from(selectionData),
- dataDisplay: selectionDisplay,
- },
+ await this.panel.postMessage('requestEditedData', {
+ data: Uint8Array.from(selectionData),
+ dataDisplay: selectionDisplay,
})
}
break
-
- case MessageCommand.replace:
+ case 'replace':
{
- const searchDataBytes = encodedStrToData(
- message.data.searchData,
- message.data.encoding
- )
- const replaceDataBytes = encodedStrToData(
- message.data.replaceData,
- message.data.encoding
- )
+ const {
+ encoding,
+ replaceStr,
+ searchStr,
+ is_case_insensitive,
+ is_reverse,
+ length,
+ offset,
+ overwriteOnly,
+ } = getRequestPayloadType<typeof command>(incomingMessage.payload[1])
+ const requestedEncoding = toEncoding(encoding)
+
+ const searchDataBytes =
+ searchStr instanceof Uint8Array
+ ? searchStr
+ : encodedStrToData(searchStr, requestedEncoding)
+ const replaceDataBytes =
+ replaceStr instanceof Uint8Array
+ ? replaceStr
+ : encodedStrToData(replaceStr, requestedEncoding)
const nextOffset = await replaceOneSession(
this.omegaSessionId,
searchDataBytes,
replaceDataBytes,
- message.data.caseInsensitive,
- message.data.isReverse,
- message.data.searchOffset,
- message.data.searchLength,
- message.data.overwriteOnly
+ is_case_insensitive,
+ is_reverse,
+ offset,
+ length,
+ overwriteOnly
)
if (nextOffset === -1) {
vscode.window.showErrorMessage('No replacement took place')
} else {
await this.sendChangesInfo()
}
- await this.panel.webview.postMessage({
- command: MessageCommand.replaceResults,
- data: {
- replacementsCount: nextOffset === -1 ? 0 : 1,
- nextOffset: nextOffset,
- searchDataBytesLength: searchDataBytes.length,
- replaceDataBytesLength: replaceDataBytes.length,
- },
+ await this.panel.postMessage('replaceResults', {
+ replacementsCount: nextOffset === -1 ? 0 : 1,
+ nextOffset: nextOffset,
+ searchDataBytesLength: searchDataBytes.length,
+ replaceDataBytesLength: replaceDataBytes.length,
})
}
break
- case MessageCommand.search:
+ // Utility Message Commands
+ case 'search':
{
- const searchDataBytes = encodedStrToData(
- message.data.searchData,
- message.data.encoding
- )
+ const {
+ encoding,
+ searchStr,
+ is_case_insensitive,
+ is_reverse,
+ length,
+ limit,
+ offset,
+ } = getRequestPayloadType<typeof command>(incomingMessage.payload[1])
+ const requestedEncoding = toEncoding(encoding)
+
+ const searchDataBytes = encodedStrToData(searchStr, requestedEncoding)
+ const searchLimit = limit ? limit + 1 : 2
+
const searchResults = await searchSession(
this.omegaSessionId,
searchDataBytes,
- message.data.caseInsensitive,
- message.data.isReverse,
- message.data.searchOffset,
- message.data.searchLength,
- message.data.limit + 1
+ is_case_insensitive,
+ is_reverse,
+ offset,
+ length,
+ searchLimit
)
+
if (searchResults.length === 0) {
vscode.window.showInformationMessage(
- `No more matches found for '${message.data.searchData}'`
+ `No more matches found for '${searchStr}'`
)
}
let overflow = false
- if (searchResults.length > message.data.limit) {
+ if (searchResults.length >= searchLimit) {
overflow = true
searchResults.pop()
}
- await this.panel.webview.postMessage({
- command: MessageCommand.searchResults,
- data: {
- searchResults: searchResults,
- searchDataBytesLength: searchDataBytes.length,
- overflow: overflow,
+ await this.panel.postMessage('search', {
+ results: searchResults,
+ byteLength: searchDataBytes.length,
+ overflow: overflow,
+ })
+ }
+ break
+ case 'profile':
+ {
+ let { length, startOffset } = getRequestPayloadType<typeof command>(
+ incomingMessage.payload[1]
+ )
+ length = length < 0 ? DATA_PROFILE_MAX_LENGTH : length
+
+ const byteProfile: number[] = await profileSession(
+ this.omegaSessionId,
+ startOffset,
+ length
+ )
+ const characterCount = await countCharacters(
+ this.omegaSessionId,
+ startOffset,
+ length
+ ).catch((err) => {
+ console.debug(err)
+ })
+ const contentTypeResponse = await getContentType(
+ this.omegaSessionId,
+ startOffset,
+ length
+ )
+ const languageResponse = await getLanguage(
+ this.omegaSessionId,
+ startOffset,
+ length,
+ characterCount!.getByteOrderMark()
+ )
Review Comment:
`countCharacters(...).catch(...)` can return `undefined`, but the rest of this handler assumes `characterCount` is always present (via `characterCount!`). If `countCharacters` fails, this will throw when calling `getByteOrderMark()` and break profiling. Either rethrow/return early on error, or provide a safe fallback BOM / characterCount object before using it.
##########
src/ext_types/messageContent.ts:
##########
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import {
+ ApplyChangesRequest,
+ ChangesInfoResponse,
+ CountResponse,
+ DFDLDataBytePos,
+ EditedDataRequest,
+ EditedDataResponse,
+ EditorOnChangeRequest,
+ EditorOnChangeResponse,
+ FileInfoResponse,
+ HeartbeatResponse,
+ NotificationRequest,
+ ProfileRequest,
+ ProfileResponse,
+ ReplaceRequest,
+ ReplaceResponse,
+ SaveAsResponse,
+ SaveResponse,
+ SaveSegmentRequest,
+ ScrollViewportRequest,
+ SearchRequest,
+ SearchResponse,
+ SetUIThemeResponse,
+ ViewportRefreshResponse,
+} from './messages'
+import { EditorRequestIds, isEditorMessageId } from './messageIds'
+
+type MessageCommands =
+ | 'clearChanges'
+ | 'applyChanges'
+ | 'editorOnChange'
+ | 'fileInfo'
+ | 'heartbeat'
+ | 'profile'
+ | 'redoChange'
+ | 'replaceResults'
+ | 'requestEditedData'
+ | 'save'
+ | 'saveAs'
+ | 'saveSegment'
+ | 'scrollViewport'
+ | 'search'
+ | 'replace'
+ | 'setUITheme'
+ | 'showMessage'
+ | 'undoChange'
+ | 'viewportRefresh'
+
+type CommandMap = {
+ [K in MessageCommands]: any
+}
+export interface MessageRequestMap extends CommandMap {
+ counts: never
+ clearChanges: never
+ applyChanges: ApplyChangesRequest
+ editorOnChange: EditorOnChangeRequest
+ fileInfo: never
+ heartbeat: never
+ profile: ProfileRequest
+ redoChange: never
+ replaceResults: never
+ requestEditedData: EditedDataRequest
+ save: never
+ saveAs: never
+ saveSegment: SaveSegmentRequest
+ scrollViewport: ScrollViewportRequest
+ search: SearchRequest
+ replace: ReplaceRequest
+ undoChange: never
+ viewportRefresh: never
+ showMessage: NotificationRequest
+}
+
+export interface MessageResponseMap extends CommandMap {
+ clearChanges: void
+ applyChanges: ChangesInfoResponse
+ editorOnChange: EditorOnChangeResponse
+ fileInfo: FileInfoResponse
+ counts: CountResponse
+ profile: ProfileResponse
+ redoChange: void
+ replaceResults: ReplaceResponse
+ requestEditedData: EditedDataResponse
+ save: SaveResponse
+ saveAs: SaveAsResponse
+ saveSegment: void
+ scrollViewport: void
+ search: SearchResponse
+ replace: ReplaceResponse
+ undoChange: void
+ viewportRefresh: ViewportRefreshResponse
+ showMessage: void
+ setUITheme: SetUIThemeResponse
Review Comment:
`MessageResponseMap` uses `void` for several no-payload responses (e.g. `clearChanges`, `redoChange`, etc.), but `PostMessageArgs` only treats `never` as “no payload”. This makes one-arg sends like `panel.postMessage('clearChanges')` inconsistent with the typing and forces unsafe casts/destructuring. Consider using `never` for no-payload response entries or updating `PostMessageArgs` to also treat `void | undefined` as no-payload.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4220497275
> @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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4469197921
Currently working on the merge conflicts resolution. As expected, due to the entire rework of message communications, there are quite a bit of conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4314809458
> It looks like sax-js switched from ISC to the BlueOak in v1.4.2.
>
> v1.4.1 - https://github.com/isaacs/sax-js/blob/v1.4.1/LICENSE
>
> v1.4.2 - https://github.com/isaacs/sax-js/blob/v1.4.2/LICENSE.md
Okay so then that makes sense. Although, should we remove the `sax-js` from the LICENSE file given that their yarn package is listed as `sax` and not `sax-js`?
I will add BlueOak to the license package data file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3238002266
##########
src/dataEditor/dataEditorClient.ts:
##########
@@ -625,108 +530,227 @@ export class DataEditorClient implements vscode.Disposable {
}
break
- case MessageCommand.saveSegment:
+ case 'applyChanges':
{
- const uri = await vscode.window.showSaveDialog({
- title: 'Save Segment',
- saveLabel: 'Save',
- })
- if (uri && uri.fsPath) {
- await this.saveFileSegment(
- uri.fsPath,
- message.data.offset,
- message.data.length
- )
+ const { offset, edited_segment, original_segment } =
+ getRequestPayloadType<typeof command>(incomingMessage.payload[1])
+ await edit(
+ this.omegaSessionId,
+ offset,
+ original_segment,
+ edited_segment
+ )
+ await this.sendChangesInfo()
+ }
+ break
+ case 'undoChange':
+ {
+ await undo(this.omegaSessionId)
+ await this.sendChangesInfo()
+ this.panel.postMessage('clearChanges')
+ }
+ break
+ case 'redoChange':
+ {
+ await redo(this.omegaSessionId)
+ await this.sendChangesInfo()
+ this.panel.postMessage('clearChanges')
+ }
+ break
+ case 'clearChanges':
+ {
+ if (
+ (await vscode.window.showInformationMessage(
+ 'Are you sure you want to revert all changes?',
+ { modal: true },
+ 'Yes',
+ 'No'
+ )) === 'Yes'
+ ) {
+ await clear(this.omegaSessionId)
+ await this.sendChangesInfo()
+ this.panel.postMessage('clearChanges')
}
}
break
-
- case MessageCommand.requestEditedData:
+ case 'requestEditedData':
{
- const [selectionData, selectionDisplay] = fillRequestData(message)
+ const payload = getRequestPayloadType<typeof command>(
+ incomingMessage.payload[1]
+ )
+ // const {} = incomingMessage.payload as MessageRequestMap['']
+ const [selectionData, selectionDisplay] = fillRequestData(payload)
- await this.panel.webview.postMessage({
- command: MessageCommand.requestEditedData,
- data: {
- data: Uint8Array.from(selectionData),
- dataDisplay: selectionDisplay,
- },
+ await this.panel.postMessage('requestEditedData', {
+ data: Uint8Array.from(selectionData),
+ dataDisplay: selectionDisplay,
})
}
break
-
- case MessageCommand.replace:
+ case 'replace':
{
- const searchDataBytes = encodedStrToData(
- message.data.searchData,
- message.data.encoding
- )
- const replaceDataBytes = encodedStrToData(
- message.data.replaceData,
- message.data.encoding
- )
+ const {
+ encoding,
+ replaceStr,
+ searchStr,
+ is_case_insensitive,
+ is_reverse,
+ length,
+ offset,
+ overwriteOnly,
+ } = getRequestPayloadType<typeof command>(incomingMessage.payload[1])
+ const requestedEncoding = toEncoding(encoding)
+
+ const searchDataBytes =
+ searchStr instanceof Uint8Array
+ ? searchStr
+ : encodedStrToData(searchStr, requestedEncoding)
+ const replaceDataBytes =
+ replaceStr instanceof Uint8Array
+ ? replaceStr
+ : encodedStrToData(replaceStr, requestedEncoding)
const nextOffset = await replaceOneSession(
this.omegaSessionId,
searchDataBytes,
replaceDataBytes,
- message.data.caseInsensitive,
- message.data.isReverse,
- message.data.searchOffset,
- message.data.searchLength,
- message.data.overwriteOnly
+ is_case_insensitive,
+ is_reverse,
+ offset,
+ length,
+ overwriteOnly
)
if (nextOffset === -1) {
vscode.window.showErrorMessage('No replacement took place')
} else {
await this.sendChangesInfo()
}
- await this.panel.webview.postMessage({
- command: MessageCommand.replaceResults,
- data: {
- replacementsCount: nextOffset === -1 ? 0 : 1,
- nextOffset: nextOffset,
- searchDataBytesLength: searchDataBytes.length,
- replaceDataBytesLength: replaceDataBytes.length,
- },
+ await this.panel.postMessage('replaceResults', {
+ replacementsCount: nextOffset === -1 ? 0 : 1,
+ nextOffset: nextOffset,
+ searchDataBytesLength: searchDataBytes.length,
+ replaceDataBytesLength: replaceDataBytes.length,
})
}
break
- case MessageCommand.search:
+ // Utility Message Commands
+ case 'search':
{
- const searchDataBytes = encodedStrToData(
- message.data.searchData,
- message.data.encoding
- )
+ const {
+ encoding,
+ searchStr,
+ is_case_insensitive,
+ is_reverse,
+ length,
+ limit,
+ offset,
+ } = getRequestPayloadType<typeof command>(incomingMessage.payload[1])
+ const requestedEncoding = toEncoding(encoding)
+
+ const searchDataBytes = encodedStrToData(searchStr, requestedEncoding)
+ const searchLimit = limit ? limit + 1 : 2
+
const searchResults = await searchSession(
this.omegaSessionId,
searchDataBytes,
- message.data.caseInsensitive,
- message.data.isReverse,
- message.data.searchOffset,
- message.data.searchLength,
- message.data.limit + 1
+ is_case_insensitive,
+ is_reverse,
+ offset,
+ length,
+ searchLimit
)
+
if (searchResults.length === 0) {
vscode.window.showInformationMessage(
- `No more matches found for '${message.data.searchData}'`
+ `No more matches found for '${searchStr}'`
)
}
let overflow = false
- if (searchResults.length > message.data.limit) {
+ if (searchResults.length >= searchLimit) {
overflow = true
searchResults.pop()
}
- await this.panel.webview.postMessage({
- command: MessageCommand.searchResults,
- data: {
- searchResults: searchResults,
- searchDataBytesLength: searchDataBytes.length,
- overflow: overflow,
+ await this.panel.postMessage('search', {
+ results: searchResults,
+ byteLength: searchDataBytes.length,
+ overflow: overflow,
+ })
+ }
+ break
+ case 'profile':
+ {
+ let { length, startOffset } = getRequestPayloadType<typeof command>(
+ incomingMessage.payload[1]
+ )
+ length = length < 0 ? DATA_PROFILE_MAX_LENGTH : length
+
+ const byteProfile: number[] = await profileSession(
+ this.omegaSessionId,
+ startOffset,
+ length
+ )
+ const characterCount = await countCharacters(
+ this.omegaSessionId,
+ startOffset,
+ length
+ ).catch((err) => {
+ console.debug(err)
+ })
+ const contentTypeResponse = await getContentType(
+ this.omegaSessionId,
+ startOffset,
+ length
+ )
+ 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(),
},
Review Comment:
`undefined` is not allowed in the new messages structure. I will throw and error in this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3245071784
##########
src/svelte/src/stores/index.ts:
##########
@@ -379,10 +371,10 @@ export const requestable = derived(
)
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
Review Comment:
I tested this and did not see this behavior. I don't think Copilot entirely understands the Svelte rune subscription model.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4313662166
@stevedlawrence What would we need to be done about the `sax` package? It uses the BlueOak-1.0.0 LICENSE, I am not finding anywhere documenting if that is not allowed or allowed so I wanted to get your input.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3238005026
##########
build/package/LICENSE:
##########
@@ -2202,6 +2202,22 @@ conditions of the following licenses.
under the terms of the GNU Lesser General Public License version 2.1
as published by the Free Software Foundation.
+- 'lightningcss' in extension/dist/ext/extension.js
+- 'lightningcss' in node_modules/vite
+ This product bundles 'lightningcss' from the above files.
+ These files are available under the MPL 2.0 license:
+
+ 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.
Review Comment:
@stevedlawrence - I'm not so good at determining the potential licensing issues. Can you verify this for me?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4216563856
> CI seems to be failing consistently and it doesn't appear to be an issue with the CI.
#1659 is the issue I created for fixing the CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4314424790
It looks like sax-js switched from ISC to the BlueOak in v1.4.2.
v1.4.1 - https://github.com/isaacs/sax-js/blob/v1.4.1/LICENSE
v1.4.2 - https://github.com/isaacs/sax-js/blob/v1.4.2/LICENSE.md
Note that this is why our license check steps mention that it's important to verify that if licenses or transitive dependencies have changed: https://cwiki.apache.org/confluence/display/DAFFODIL/Scala+Steward
We might want to update that page so it's not specific to scala steward since it really is more about licenses in general.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4315047317
> > So I would check if there are two different versions of sax-js in the dependencies.
>
> @shanedell - Wouldn't the latest version's license supersede previous versions?
@stricklandrbls Possibly, I am thinking that yarn lets you pin multiple version but that might not be right. So the package name is not `sax-js` you are right there, its actually name `sax`. But the repo name is `sax-js` and that might be where that came from. But from looking at yarn.lock it has `sax@^1.2.4` so you should only need to list `sax` once with the BlueOak license
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4309664270
@shanedell @hdalsania I'm not sure what else to add to the licensing files regarding the `sax` dependency license (BlueOak-1.0.0). The CI looks to be resolved for the `lightningcss` packages that had the MPL license but there's still errors for `sax`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3245070633
##########
src/svelte/src/components/Header/fieldsets/SearchReplace.svelte:
##########
@@ -403,7 +388,7 @@ limitations under the License.
<Button
fn={replace}
description="Replace the current match"
- disabledBy={!replaceable || replaceErrDisplay}
+ isDisabled={!replaceable || replaceErrDisplay}
>
Review Comment:
I tested this and did not see this behavior. I don't think Copilot entirely understands the Svelte rune subscription model.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4314825573
> So I would check if there are two different versions of sax-js in the dependencies.
@shanedell - Wouldn't the latest version's license supersede previous versions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3182718071
##########
src/dataEditor/ui/svelteWebviewInitializer.ts:
##########
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as vscode from 'vscode'
+import * as fs from 'fs'
+import { MessageResponseMap, PostMessageArgs } from 'ext_types'
+import { DisplayState } from './displayState'
+
+export interface DataEditorUI extends vscode.Disposable {
+ readonly displayState: DisplayState
+ dispose(): void
+ setTitle(title: string): void
+ getMsgId(): string
+ postMessage<K extends keyof MessageResponseMap>(
+ ...payload: PostMessageArgs<MessageResponseMap, K>
+ ): void
+ reveal: vscode.WebviewPanel['reveal']
+ onDidReceiveMessage(listener: (e: any) => void, thisArg?: any): void
+ onDidDispose(listener: (e: any) => void): void
+}
+class SvelteUIWebviewPanel implements DataEditorUI {
+ static readonly uiViewId: string = 'dataEditor'
+ readonly displayState: DisplayState = new DisplayState()
+
+ constructor(
+ private vscodePanel: vscode.WebviewPanel,
+ private messengerId: string = ''
+ ) {}
+ dispose() {
+ this.vscodePanel.dispose()
+ }
+ getMsgId() {
+ return this.messengerId
+ }
+ setMsgId(id: string) {
+ if (this.messengerId !== '')
+ throw new Error('Cannot reassign messenger id after initialization.')
+
+ this.messengerId = id
+ }
+ setTitle(title: string) {
+ this.vscodePanel.title = title
+ }
+
+ postMessage<K extends keyof MessageResponseMap>(
+ ...msg: PostMessageArgs<MessageResponseMap, K>
+ ) {
+ const [type, payload] = msg as [K, MessageResponseMap[K]]
+ this.vscodePanel.webview.postMessage({
+ command: type,
+ id: this.messengerId,
+ data: payload,
+ })
+ }
+
+ public onDidReceiveMessage(listener: (e: any) => void, thisArg?: any) {
+ this.vscodePanel.webview.onDidReceiveMessage(listener, thisArg)
+ }
+
+ public readonly reveal = () => {
+ this.vscodePanel.reveal()
+ }
+
+ public onDidDispose(listener: (e: any) => void) {
+ this.vscodePanel.onDidDispose(listener)
+ }
+}
+
+export type SveltePanelArgs = {
+ title: string
+ column: vscode.ViewColumn
+ uiMsgId: string
+}
+
+interface SvelteInitializerState {
+ isContextValid: boolean
+ getExtensionCtx(): vscode.ExtensionContext
+ getSvelteWebviewInitializer(): SvelteWebviewInitializer
+}
+const InvalidInitializerState: SvelteInitializerState = {
+ isContextValid: false,
+ getSvelteWebviewInitializer: () => {
+ throw 'SvelteWebviewInitializer is in an invalid state'
+ },
+ getExtensionCtx: function (): vscode.ExtensionContext {
+ throw 'SvelteWebviewInitializer is in an invalid state'
+ },
+}
+
+const ValidInitializerState: SvelteInitializerState = {
+ isContextValid: true,
+ getSvelteWebviewInitializer: () => {
+ return SvelteWebviewMgr
+ },
+ getExtensionCtx: () => {
+ return ExtensionContextRef
+ },
+}
+
+const UI_MSG_ID_MAX_LEN = 32
+
+class SvelteWebviewInitializer {
+ static state: SvelteInitializerState = InvalidInitializerState
+ constructor() {}
+ formatMsgId(id: string, isDFDLDebugAttached: boolean = false) {
+ let idStr = id.substring(0, UI_MSG_ID_MAX_LEN)
+ idStr = idStr.replaceAll(' ', '_')
+ return isDFDLDebugAttached ? 'dfdl-' + idStr : idStr
+ }
Review Comment:
The messenger id is derived only from the panel title (and then truncated to 32 chars), so two editors opened on different files with the same basename—or long basenames sharing the same prefix—end up with the same id. Because the new routing logic filters purely on this id, those editors will still receive each other's messages.
##########
src/dataEditor/ui/svelteWebviewInitializer.ts:
##########
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as vscode from 'vscode'
+import * as fs from 'fs'
+import { MessageResponseMap, PostMessageArgs } from 'ext_types'
+import { DisplayState } from './displayState'
+
+export interface DataEditorUI extends vscode.Disposable {
+ readonly displayState: DisplayState
+ dispose(): void
+ setTitle(title: string): void
+ getMsgId(): string
+ postMessage<K extends keyof MessageResponseMap>(
+ ...payload: PostMessageArgs<MessageResponseMap, K>
+ ): void
+ reveal: vscode.WebviewPanel['reveal']
+ onDidReceiveMessage(listener: (e: any) => void, thisArg?: any): void
+ onDidDispose(listener: (e: any) => void): void
+}
+class SvelteUIWebviewPanel implements DataEditorUI {
+ static readonly uiViewId: string = 'dataEditor'
+ readonly displayState: DisplayState = new DisplayState()
+
+ constructor(
+ private vscodePanel: vscode.WebviewPanel,
+ private messengerId: string = ''
+ ) {}
+ dispose() {
+ this.vscodePanel.dispose()
+ }
+ getMsgId() {
+ return this.messengerId
+ }
+ setMsgId(id: string) {
+ if (this.messengerId !== '')
+ throw new Error('Cannot reassign messenger id after initialization.')
+
+ this.messengerId = id
+ }
+ setTitle(title: string) {
+ this.vscodePanel.title = title
+ }
+
+ postMessage<K extends keyof MessageResponseMap>(
+ ...msg: PostMessageArgs<MessageResponseMap, K>
+ ) {
+ const [type, payload] = msg as [K, MessageResponseMap[K]]
+ this.vscodePanel.webview.postMessage({
+ command: type,
+ id: this.messengerId,
+ data: payload,
+ })
+ }
+
+ public onDidReceiveMessage(listener: (e: any) => void, thisArg?: any) {
+ this.vscodePanel.webview.onDidReceiveMessage(listener, thisArg)
+ }
+
+ public readonly reveal = () => {
+ this.vscodePanel.reveal()
+ }
+
+ public onDidDispose(listener: (e: any) => void) {
+ this.vscodePanel.onDidDispose(listener)
+ }
+}
+
+export type SveltePanelArgs = {
+ title: string
+ column: vscode.ViewColumn
+ uiMsgId: string
+}
+
+interface SvelteInitializerState {
+ isContextValid: boolean
+ getExtensionCtx(): vscode.ExtensionContext
+ getSvelteWebviewInitializer(): SvelteWebviewInitializer
+}
+const InvalidInitializerState: SvelteInitializerState = {
+ isContextValid: false,
+ getSvelteWebviewInitializer: () => {
+ throw 'SvelteWebviewInitializer is in an invalid state'
+ },
+ getExtensionCtx: function (): vscode.ExtensionContext {
+ throw 'SvelteWebviewInitializer is in an invalid state'
+ },
+}
+
+const ValidInitializerState: SvelteInitializerState = {
+ isContextValid: true,
+ getSvelteWebviewInitializer: () => {
+ return SvelteWebviewMgr
+ },
+ getExtensionCtx: () => {
+ return ExtensionContextRef
+ },
+}
+
+const UI_MSG_ID_MAX_LEN = 32
+
+class SvelteWebviewInitializer {
+ static state: SvelteInitializerState = InvalidInitializerState
+ constructor() {}
+ formatMsgId(id: string, isDFDLDebugAttached: boolean = false) {
+ let idStr = id.substring(0, UI_MSG_ID_MAX_LEN)
+ idStr = idStr.replaceAll(' ', '_')
+ return isDFDLDebugAttached ? 'dfdl-' + idStr : idStr
+ }
+ createSveltePanel(args: SveltePanelArgs): SvelteUIWebviewPanel {
+ const opts = this.getWebviewOptions()
+ const ret = vscode.window.createWebviewPanel(
+ SvelteUIWebviewPanel.uiViewId,
+ args.title,
+ args.column,
+ {
+ ...opts,
+ }
+ )
+ this.setHtmlContent(ret, args.uiMsgId)
+
+ return new SvelteUIWebviewPanel(ret, args.uiMsgId)
+ }
+
+ // get the HTML content for the webview
+ private setHtmlContent(vsPanel: vscode.WebviewPanel, msgId: string) {
+ let vsWebview = vsPanel.webview
+ const nonce = this.getNonce()
+
+ const scriptUri = this.getResourceUri('js', ExtensionContextRef, (uri) => {
+ return vsWebview.asWebviewUri(uri)
+ })
+ const stylesUri = this.getResourceUri('css', ExtensionContextRef, (uri) => {
+ return vsWebview.asWebviewUri(uri)
+ })
+
+ let indexHTML = this.injectNonce(
+ this.getIndexHTML(ExtensionContextRef),
+ vsWebview,
+ nonce,
+ scriptUri
+ )!
+ indexHTML = indexHTML
+ .replace(/src="\.\/index.js"/, `src="${scriptUri.toString()}"`)
+ .replace(/href="\.\/style.css"/, `href="${stylesUri.toString()}"`)
+ .replaceAll(/nonce="__nonce__"/g, `nonce="${nonce}"`)
+ .replace('__extension_msg_id__', msgId)
+
+ vsWebview.html = indexHTML
+ }
Review Comment:
`msgId` is injected straight into an HTML attribute without escaping. A filename containing `"`, `<`, or `&` will produce invalid HTML here and can break the webview bootstrap for perfectly valid workspace files.
##########
src/dataEditor/ui/displayState.ts:
##########
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import * as vscode from 'vscode'
+
+interface DisplayStateAttributes {
+ title: string
+ encoding: BufferEncoding
+ colorTheme: vscode.ColorThemeKind
+}
+
+export class DisplayState {
+ private attributes: DisplayStateAttributes = {
+ title: '',
+ encoding: 'hex',
+ colorTheme: vscode.window.activeColorTheme.kind,
+ }
+
+ constructor() {}
+
+ update<K extends keyof DisplayStateAttributes>(
+ attribute: K,
+ value: DisplayStateAttributes[K]
+ ): void {
+ this.attributes[attribute] = value
+ }
+
+ get<K extends keyof DisplayStateAttributes>(
+ attribute: K
+ ): DisplayStateAttributes[K] {
+ return this.attributes[attribute]
+ }
+
+ onColorThemeChanged(
+ listener: (theme: DisplayStateAttributes['colorTheme']) => any
+ ) {
+ vscode.window.onDidChangeActiveColorTheme((newTheme) => {
Review Comment:
This only subscribes to future theme changes; it never delivers the current theme to a newly opened editor. The Svelte UI defaults to dark, so users opening the data editor under a light VS Code theme now get the wrong colors until they manually toggle themes.
##########
src/svelte/src/stores/index.ts:
##########
@@ -379,10 +371,10 @@ export const requestable = derived(
)
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
Review Comment:
This derived store now also reads `isRegularSizedFile()` without subscribing to `fileMetricsState.computedSize`. After the file size changes, it can keep returning the old branch and send the wrong original segment back in `applyChanges` (entire viewport vs selected slice).
##########
build/package/LICENSE:
##########
@@ -2202,6 +2202,22 @@ conditions of the following licenses.
under the terms of the GNU Lesser General Public License version 2.1
as published by the Free Software Foundation.
+- 'lightningcss' in extension/dist/ext/extension.js
+- 'lightningcss' in node_modules/vite
+ This product bundles 'lightningcss' from the above files.
+ These files are available under the MPL 2.0 license:
+
+ 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.
Review Comment:
This new license blurb also says `sax` is part of the 'Saxon product', which is clearly incorrect. The generated LICENSE should not ship dependency attributions copied from an unrelated package.
##########
build/package/LICENSE:
##########
@@ -2202,6 +2202,22 @@ conditions of the following licenses.
under the terms of the GNU Lesser General Public License version 2.1
as published by the Free Software Foundation.
+- 'lightningcss' in extension/dist/ext/extension.js
+- 'lightningcss' in node_modules/vite
+ This product bundles 'lightningcss' from the above files.
+ These files are available under the MPL 2.0 license:
+
+ 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.
Review Comment:
This new license blurb still says the code is part of the 'Saxon product', which is unrelated to `lightningcss`. Shipping the generated LICENSE with the wrong attribution makes the package metadata inaccurate.
##########
src/svelte/src/components/ServerMetrics/ServerMetrics.svelte:
##########
@@ -66,43 +68,46 @@ limitations under the License.
? `${seconds} second`
: `${seconds} seconds`
}
-
- window.addEventListener('message', (msg) => {
- switch (msg.data.command) {
- case MessageCommand.heartbeat:
- heartbeat.latency = msg.data.data.latency
- heartbeat.serverCpuLoadAverage = msg.data.data.serverCpuLoadAverage
- heartbeat.serverTimestamp = msg.data.data.serverTimestamp
- heartbeat.serverUptime = msg.data.data.serverUptime
- heartbeat.serverUsedMemory = msg.data.data.serverUsedMemory
- heartbeat.sessionCount = msg.data.data.sessionCount
- heartbeat.omegaEditPort = msg.data.data.serverInfo.omegaEditPort
- heartbeat.serverVersion = msg.data.data.serverInfo.serverVersion
- heartbeat.serverHostname = msg.data.data.serverInfo.serverHostname
- heartbeat.serverProcessId = msg.data.data.serverInfo.serverProcessId
- heartbeat.jvmVersion = msg.data.data.serverInfo.jvmVersion
- heartbeat.jvmVendor = msg.data.data.serverInfo.jvmVendor
- heartbeat.jvmPath = msg.data.data.serverInfo.jvmPath
- heartbeat.availableProcessors =
- msg.data.data.serverInfo.availableProcessors
-
- // set the serverTimestamp to 0 after 5 seconds of no heartbeat to indicate that no heartbeat has been received
- clearTimeout(timerId)
- timerId = setTimeout(() => {
- heartbeat.serverTimestamp = 0
- }, 5000)
- break
- }
+ addListener('heartbeat', (data) => {
+ const {
+ latency,
+ port,
+ serverCpuLoadAverage,
+ serverTimestamp,
+ serverUptime,
+ serverUsedMemory,
+ sessionCount,
+ serverInfo,
+ } = data
+ heartbeat.latency = latency
+ heartbeat.port = port
Review Comment:
The state object still defines `omegaEditPort`, but the new code reads and writes `heartbeat.port`. That property does not exist on the inferred type, so `svelte-check`/TypeScript now reports errors in this component.
##########
src/svelte/src/components/Header/fieldsets/SearchReplace.svelte:
##########
@@ -403,7 +388,7 @@ limitations under the License.
<Button
fn={replace}
description="Replace the current match"
- disabledBy={!replaceable || replaceErrDisplay}
+ isDisabled={!replaceable || replaceErrDisplay}
>
Review Comment:
`replaceable` is a store object, so `!replaceable` is always `false` here. This leaves the in-session Replace button enabled even when `$replaceable` has gone false (for example while a request is in flight or the current inputs are invalid).
##########
src/dataEditor/dataEditorClient.ts:
##########
@@ -625,108 +530,227 @@ export class DataEditorClient implements vscode.Disposable {
}
break
- case MessageCommand.saveSegment:
+ case 'applyChanges':
{
- const uri = await vscode.window.showSaveDialog({
- title: 'Save Segment',
- saveLabel: 'Save',
- })
- if (uri && uri.fsPath) {
- await this.saveFileSegment(
- uri.fsPath,
- message.data.offset,
- message.data.length
- )
+ const { offset, edited_segment, original_segment } =
+ getRequestPayloadType<typeof command>(incomingMessage.payload[1])
+ await edit(
+ this.omegaSessionId,
+ offset,
+ original_segment,
+ edited_segment
+ )
+ await this.sendChangesInfo()
+ }
+ break
+ case 'undoChange':
+ {
+ await undo(this.omegaSessionId)
+ await this.sendChangesInfo()
+ this.panel.postMessage('clearChanges')
+ }
+ break
+ case 'redoChange':
+ {
+ await redo(this.omegaSessionId)
+ await this.sendChangesInfo()
+ this.panel.postMessage('clearChanges')
+ }
+ break
+ case 'clearChanges':
+ {
+ if (
+ (await vscode.window.showInformationMessage(
+ 'Are you sure you want to revert all changes?',
+ { modal: true },
+ 'Yes',
+ 'No'
+ )) === 'Yes'
+ ) {
+ await clear(this.omegaSessionId)
+ await this.sendChangesInfo()
+ this.panel.postMessage('clearChanges')
}
}
break
-
- case MessageCommand.requestEditedData:
+ case 'requestEditedData':
{
- const [selectionData, selectionDisplay] = fillRequestData(message)
+ const payload = getRequestPayloadType<typeof command>(
+ incomingMessage.payload[1]
+ )
+ // const {} = incomingMessage.payload as MessageRequestMap['']
+ const [selectionData, selectionDisplay] = fillRequestData(payload)
- await this.panel.webview.postMessage({
- command: MessageCommand.requestEditedData,
- data: {
- data: Uint8Array.from(selectionData),
- dataDisplay: selectionDisplay,
- },
+ await this.panel.postMessage('requestEditedData', {
+ data: Uint8Array.from(selectionData),
+ dataDisplay: selectionDisplay,
})
}
break
-
- case MessageCommand.replace:
+ case 'replace':
{
- const searchDataBytes = encodedStrToData(
- message.data.searchData,
- message.data.encoding
- )
- const replaceDataBytes = encodedStrToData(
- message.data.replaceData,
- message.data.encoding
- )
+ const {
+ encoding,
+ replaceStr,
+ searchStr,
+ is_case_insensitive,
+ is_reverse,
+ length,
+ offset,
+ overwriteOnly,
+ } = getRequestPayloadType<typeof command>(incomingMessage.payload[1])
+ const requestedEncoding = toEncoding(encoding)
+
+ const searchDataBytes =
+ searchStr instanceof Uint8Array
+ ? searchStr
+ : encodedStrToData(searchStr, requestedEncoding)
+ const replaceDataBytes =
+ replaceStr instanceof Uint8Array
+ ? replaceStr
+ : encodedStrToData(replaceStr, requestedEncoding)
const nextOffset = await replaceOneSession(
this.omegaSessionId,
searchDataBytes,
replaceDataBytes,
- message.data.caseInsensitive,
- message.data.isReverse,
- message.data.searchOffset,
- message.data.searchLength,
- message.data.overwriteOnly
+ is_case_insensitive,
+ is_reverse,
+ offset,
+ length,
+ overwriteOnly
)
if (nextOffset === -1) {
vscode.window.showErrorMessage('No replacement took place')
} else {
await this.sendChangesInfo()
}
- await this.panel.webview.postMessage({
- command: MessageCommand.replaceResults,
- data: {
- replacementsCount: nextOffset === -1 ? 0 : 1,
- nextOffset: nextOffset,
- searchDataBytesLength: searchDataBytes.length,
- replaceDataBytesLength: replaceDataBytes.length,
- },
+ await this.panel.postMessage('replaceResults', {
+ replacementsCount: nextOffset === -1 ? 0 : 1,
+ nextOffset: nextOffset,
+ searchDataBytesLength: searchDataBytes.length,
+ replaceDataBytesLength: replaceDataBytes.length,
})
}
break
- case MessageCommand.search:
+ // Utility Message Commands
+ case 'search':
{
- const searchDataBytes = encodedStrToData(
- message.data.searchData,
- message.data.encoding
- )
+ const {
+ encoding,
+ searchStr,
+ is_case_insensitive,
+ is_reverse,
+ length,
+ limit,
+ offset,
+ } = getRequestPayloadType<typeof command>(incomingMessage.payload[1])
+ const requestedEncoding = toEncoding(encoding)
+
+ const searchDataBytes = encodedStrToData(searchStr, requestedEncoding)
+ const searchLimit = limit ? limit + 1 : 2
+
const searchResults = await searchSession(
this.omegaSessionId,
searchDataBytes,
- message.data.caseInsensitive,
- message.data.isReverse,
- message.data.searchOffset,
- message.data.searchLength,
- message.data.limit + 1
+ is_case_insensitive,
+ is_reverse,
+ offset,
+ length,
+ searchLimit
)
+
if (searchResults.length === 0) {
vscode.window.showInformationMessage(
- `No more matches found for '${message.data.searchData}'`
+ `No more matches found for '${searchStr}'`
)
}
let overflow = false
- if (searchResults.length > message.data.limit) {
+ if (searchResults.length >= searchLimit) {
overflow = true
searchResults.pop()
}
- await this.panel.webview.postMessage({
- command: MessageCommand.searchResults,
- data: {
- searchResults: searchResults,
- searchDataBytesLength: searchDataBytes.length,
- overflow: overflow,
+ await this.panel.postMessage('search', {
+ results: searchResults,
+ byteLength: searchDataBytes.length,
+ overflow: overflow,
+ })
+ }
+ break
+ case 'profile':
+ {
+ let { length, startOffset } = getRequestPayloadType<typeof command>(
+ incomingMessage.payload[1]
+ )
+ length = length < 0 ? DATA_PROFILE_MAX_LENGTH : length
+
+ const byteProfile: number[] = await profileSession(
+ this.omegaSessionId,
+ startOffset,
+ length
+ )
+ const characterCount = await countCharacters(
+ this.omegaSessionId,
+ startOffset,
+ length
+ ).catch((err) => {
+ console.debug(err)
+ })
+ const contentTypeResponse = await getContentType(
+ this.omegaSessionId,
+ startOffset,
+ length
+ )
+ 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(),
},
Review Comment:
If `countCharacters()` rejects, the catch block only logs it and leaves `characterCount` undefined, but the next lines still dereference `characterCount!` several times. That turns a recoverable profiling error into a runtime exception in the message handler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4315895085
@scholarsmate @shanedell @hdalsania @CoverRyan - I believe this PR is in order and is ready for a re-review. Minor changes were made from the Copilot suggestions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3177357862
##########
build/package/LICENSE:
##########
@@ -2202,6 +2202,22 @@ conditions of the following licenses.
under the terms of the GNU Lesser General Public License version 2.1
as published by the Free Software Foundation.
+- 'lightningcss' in extension/dist/ext/extension.js
+- 'lightningcss' in node_modules/vite
+ This product bundles 'lightningcss' from the above files.
+ These files are available under the MPL 2.0 license:
+
+ 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
Review Comment:
The bundled-license preamble you added for both `lightningcss` and `sax` says these files are part of 'the Saxon product', which is factually unrelated to either dependency. Shipping the generated LICENSE with incorrect package descriptions makes the bundled-license report inaccurate.
##########
src/svelte/src/components/DataMetrics/DataMetrics.svelte:
##########
@@ -279,39 +273,39 @@ limitations under the License.
}
onMount(() => {
- // Handle messages sent from the extension to the webview
- window.addEventListener('message', (msg) => {
- switch (msg.data.command) {
- case MessageCommand.profile:
- numAscii = msg.data.data.numAscii as number
- byteProfile = msg.data.data.byteProfile as number[]
- language = msg.data.data.language as string
- contentType = msg.data.data.contentType as string
-
- // character count data
- characterCountData.byteOrderMark = msg.data.data.characterCount
- .byteOrderMark as string
- characterCountData.byteOrderMarkBytes = msg.data.data.characterCount
- .byteOrderMarkBytes as number
- characterCountData.singleByteCount = msg.data.data.characterCount
- .singleByteCount as number
- characterCountData.doubleByteCount = msg.data.data.characterCount
- .doubleByteCount as number
- characterCountData.tripleByteCount = msg.data.data.characterCount
- .tripleByteCount as number
- characterCountData.quadByteCount = msg.data.data.characterCount
- .quadByteCount as number
- characterCountData.invalidBytes = msg.data.data.characterCount
- .invalidBytes as number
-
- setStatusMessage(
- `Profiled bytes from ${startOffset} to ${startOffset + length}`
- )
- break
- default:
- break // do nothing
- }
+ addListener('profile', (data) => {
+ const {
+ numAscii,
+ byteProfile,
+ characterCount,
+ contentType,
+ language,
+ length,
+ startOffset,
+ } = data
+ asciiCount = numAscii
+ profileBytes = byteProfile
+ content = contentType
+ lang = language
+ // character count data
+ characterCountData.byteOrderMark = characterCount.byteOrderMark as string
+ characterCountData.byteOrderMarkBytes =
+ characterCount.byteOrderMarkBytes as number
+ characterCountData.singleByteCount =
+ characterCount.singleByteCount as number
+ characterCountData.doubleByteCount =
+ characterCount.doubleByteCount as number
+ characterCountData.tripleByteCount =
+ characterCount.tripleByteCount as number
+ characterCountData.quadByteCount = characterCount.quadByteCount as number
+ characterCountData.invalidBytes = characterCount.invalidBytes as number
+
+ setStatusMessage(
+ `Profiled bytes from ${startOffset} to ${startOffset + length}`
+ )
})
+ length = length < 0 ? DATA_PROFILE_MAX_LENGTH : length
Review Comment:
`length` is now only defaulted when it is negative, but the parent still initializes the profiler with `length = 0`. On first open this sends `profile { startOffset: 0, length: 0 }`, so the profiler requests an empty range instead of profiling the file's initial segment.
##########
src/svelte/src/components/Header/fieldsets/Settings.svelte:
##########
@@ -28,30 +28,45 @@ limitations under the License.
import FlexContainer from '../../layouts/FlexContainer.svelte'
import { UIThemeCSSClass } from '../../../utilities/colorScheme'
import ViewportVisibilityIcon from '../../Icons/ViewportVisibilityIcon.svelte'
- import { MessageCommand } from '../../../utilities/message'
+ import {
+ fileMetricsState,
+ isRegularSizedFile,
+ saveable,
+ } from './FileMetrics.svelte.ts'
+
+ const { addListener } = getUIMessegnerCtx()
/* DEBUG_ONLY_START */
import { getDebugVarContext } from '../../Debug/'
+ import { getUIMessegnerCtx } from 'utilities/messageContext.svelte.ts'
Review Comment:
`getUIMessegnerCtx()` is called outside the `DEBUG_ONLY` block, but its import sits inside that block. The production build strips lines 39-64, so the import disappears while line 38 remains, which leaves `getUIMessegnerCtx` undefined and breaks the non-debug Svelte build.
##########
src/svelte/src/components/DataDisplays/CustomByteDisplay/DataLineFeed.svelte:
##########
@@ -562,36 +554,36 @@ limitations under the License.
}
window.addEventListener('keydown', navigation_keydown_event)
- window.addEventListener('message', (msg) => {
- switch (msg.data.command) {
- case MessageCommand.viewportRefresh:
- if (awaitViewportSeek) {
- awaitViewportSeek = false
- $dataFeedLineTop = Math.max(
- 0,
- Math.min(lineTopMaxViewport, $dataFeedLineTop)
- )
- if ($selectionDataStore.active)
- selectedByteElement = document.getElementById(
- $selectedByte.offset.toString()
- ) as HTMLDivElement
- }
- break
- case 'daffodil.data':
- const { bytePos1b } = msg.data.data
- if (!bytePosIsDisplayable(bytePos1b - 1)) {
- $seekOffsetInput = bytePos1b.toString(addressRadix)
- eventDispatcher('seek')
- }
- $dfdlBytePos = bytePos1b - 1
- break
+
+ addListener('viewportRefresh', (data) => {
+ if (awaitViewportSeek) {
+ awaitViewportSeek = false
+ $dataFeedLineTop = Math.max(
+ 0,
+ Math.min(lineTopMaxViewport, $dataFeedLineTop)
+ )
+ if ($selectionDataStore.active)
+ selectedByteElement = document.getElementById(
+ $selectedByte.offset.toString()
+ ) as HTMLDivElement
}
})
+ if (isUIDebugAttached(getUIMsgId())) {
+ addListener('bytePos1b', (data) => {
+ const { bytePos1b } = data
+ if (!bytePosIsDisplayable(bytePos1b - 1)) {
+ $seekOffsetInput = bytePos1b.toString(addressRadix)
+ eventDispatcher('seek')
+ }
+ $dfdlBytePos = bytePos1b - 1
+ })
+ }
Review Comment:
This listener is only attached when the initial UI id already contains the `dfdl-` prefix. That means an editor opened before a DFDL session starts will never subscribe to `bytePos1b`, even after the matching debug session becomes active later, so live byte-position tracking regresses unless the editor is opened after debugging has already begun.
##########
src/dataEditor/dataEditorClient.ts:
##########
@@ -846,16 +870,18 @@ export class DataEditorClient implements vscode.Disposable {
}
if (saved) {
- this.fileToEdit = fileToSave
const fileSize = await getComputedFileSize(this.omegaSessionId)
- await this.panel.webview.postMessage({
- command: MessageCommand.fileInfo,
- data: {
+ if (this.fileToEdit !== fileToSave) {
+ this.fileToEdit = fileToSave
+ this.panel.postMessage('saveAs', {
computedFileSize: fileSize,
- diskFileSize: fileSize,
- fileName: fileToSave,
- },
- })
+ newFilePath: this.fileToEdit,
+ })
Review Comment:
When `Save As` changes `this.fileToEdit`, the `OPEN_EDITORS` index is not updated from the old path to the new one. After this, `isActive()` starts returning `false`, disposing the editor deletes the wrong key, and reopening the original file is incorrectly treated as a duplicate because the stale map entry is left behind.
##########
src/svelte/src/App.svelte:
##########
@@ -64,21 +56,34 @@ limitations under the License.
} from './components/DataDisplays/CustomByteDisplay/BinaryData'
import { byte_count_divisible_offset } from './utilities/display'
import Help from './components/layouts/Help.svelte'
- import { viewportByteIndicators } from 'utilities/highlights'
+ import { EditByteModes, type BytesPerRow } from 'ext_types'
+ import { VIEWPORT_SCROLL_INCREMENT } from './stores/configuration'
+ import { vscode } from './utilities/vscode'
+ import { isRegularSizedFile } from './components/Header/fieldsets/FileMetrics.svelte.ts'
+ import { viewportByteIndicators } from 'utilities/highlights.ts'
+ import {
+ getUIMessegnerCtx,
+ setUIMessegnerCtx,
+ } from 'utilities/messageContext.svelte.ts'
+
+ let { mountTarget }: { mountTarget: HTMLElement } = $props<{
+ mountTarget: HTMLElement
+ }>()
+
+ const uiMsgId = mountTarget.attributes['extension_msg_id'].value
+ setUIMessegnerCtx(vscode.getMessenger(uiMsgId))
+ const { addListener, postMessage } = getUIMessegnerCtx()
Review Comment:
`App.svelte` now assumes a `mountTarget` prop is always present and immediately dereferences `mountTarget.attributes[...]`, but the debug entrypoint still renders `<App />` with no props (`src/svelte/src/App.debug.svelte:22-24`). Running the data editor in `DEBUG_DATAEDITOR=on` will therefore crash before the UI finishes mounting.
##########
src/svelte/src/App.svelte:
##########
@@ -64,21 +56,34 @@ limitations under the License.
} from './components/DataDisplays/CustomByteDisplay/BinaryData'
import { byte_count_divisible_offset } from './utilities/display'
import Help from './components/layouts/Help.svelte'
- import { viewportByteIndicators } from 'utilities/highlights'
+ import { EditByteModes, type BytesPerRow } from 'ext_types'
+ import { VIEWPORT_SCROLL_INCREMENT } from './stores/configuration'
+ import { vscode } from './utilities/vscode'
+ import { isRegularSizedFile } from './components/Header/fieldsets/FileMetrics.svelte.ts'
+ import { viewportByteIndicators } from 'utilities/highlights.ts'
+ import {
+ getUIMessegnerCtx,
+ setUIMessegnerCtx,
+ } from 'utilities/messageContext.svelte.ts'
+
+ let { mountTarget }: { mountTarget: HTMLElement } = $props<{
+ mountTarget: HTMLElement
+ }>()
+
+ const uiMsgId = mountTarget.attributes['extension_msg_id'].value
+ setUIMessegnerCtx(vscode.getMessenger(uiMsgId))
+ const { addListener, postMessage } = getUIMessegnerCtx()
Review Comment:
`getUIMsgId()` is used later to decide whether the UI should subscribe to `bytePos1b`, but the new app initialization never writes the extracted `uiMsgId` into this state. As a result `getUIMsgId()` stays `''` and the DFDL byte-position listener is never registered, so debug byte-position highlighting stops working.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "lrbarber (via GitHub)" <gi...@apache.org>.
lrbarber commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4478336740
I am unable to get this to run. Seeing the following crash message.
The first picture occurred with a breakpoint set. Thinking that might be related to the problem, I deleted all breakpoints and tried again with those results in the second picture.
<img width="1446" height="893" alt="image" src="https://github.com/user-attachments/assets/5465e685-3cc1-41c0-907c-928d90abf9e1" />
<img width="1449" height="824" alt="image" src="https://github.com/user-attachments/assets/6e11904b-f06c-4294-a7f0-8d2566e83b08" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4478663266
> I am unable to get this to run. Seeing the following crash message.
>
> The first picture occurred with a breakpoint set. Thinking that might be related to the problem, I deleted all breakpoints and tried again with those results in the second picture.
>
> <img width="1446" height="893" alt="image" src="https://github.com/user-attachments/assets/5465e685-3cc1-41c0-907c-928d90abf9e1" />
>
>
>
> <img width="1449" height="824" alt="image" src="https://github.com/user-attachments/assets/6e11904b-f06c-4294-a7f0-8d2566e83b08" />
>
>
Yeah I'm still working on the migration but committing so I don't lose progress. There were way too many merge conflicts with the OE version 2 upgrade branch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3238006685
##########
build/package/LICENSE:
##########
@@ -2202,6 +2202,22 @@ conditions of the following licenses.
under the terms of the GNU Lesser General Public License version 2.1
as published by the Free Software Foundation.
+- 'lightningcss' in extension/dist/ext/extension.js
+- 'lightningcss' in node_modules/vite
+ This product bundles 'lightningcss' from the above files.
+ These files are available under the MPL 2.0 license:
+
+ 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.
Review Comment:
@stevedlawrence - Same here, please.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4314586257
Yep, if two different versions of the same library are included and they have different licenses, then we need to mention them both. But I would think npm/yarn would only include the latest, especially since 1.4.1 to 1.4.2 is a patch version bump. So in theory they should be compatible and it should be able to evict the older version without problem. But maybe it does have some way to support multiple versions of the same library? I'm not familar with how yarn/npm do all their dependency magic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3139740509
##########
src/svelte/src/components/DataMetrics/DataMetrics.svelte:
##########
@@ -279,41 +276,41 @@ limitations under the License.
}
onMount(() => {
- // Handle messages sent from the extension to the webview
- window.addEventListener('message', (msg) => {
- switch (msg.data.command) {
- case MessageCommand.profile:
- numAscii = msg.data.data.numAscii as number
- byteProfile = msg.data.data.byteProfile as number[]
- language = msg.data.data.language as string
- contentType = msg.data.data.contentType as string
-
- // character count data
- characterCountData.byteOrderMark = msg.data.data.characterCount
- .byteOrderMark as string
- characterCountData.byteOrderMarkBytes = msg.data.data.characterCount
- .byteOrderMarkBytes as number
- characterCountData.singleByteCount = msg.data.data.characterCount
- .singleByteCount as number
- characterCountData.doubleByteCount = msg.data.data.characterCount
- .doubleByteCount as number
- characterCountData.tripleByteCount = msg.data.data.characterCount
- .tripleByteCount as number
- characterCountData.quadByteCount = msg.data.data.characterCount
- .quadByteCount as number
- characterCountData.invalidBytes = msg.data.data.characterCount
- .invalidBytes as number
-
- setStatusMessage(
- `Profiled bytes from ${startOffset} to ${startOffset + length}`
- )
- break
- default:
- break // do nothing
- }
+ addListener('profile', (data) => {
+ const {
+ numAscii,
+ byteProfile,
+ characterCount,
+ contentType,
+ language,
+ length,
+ startOffset,
+ } = data
+ asciiCount = numAscii
+ profileBytes = byteProfile
+ content = contentType
+ lang = language
+ // character count data
+ characterCountData.byteOrderMark = characterCount.byteOrderMark as string
+ characterCountData.byteOrderMarkBytes =
+ characterCount.byteOrderMarkBytes as number
+ characterCountData.singleByteCount =
+ characterCount.singleByteCount as number
+ characterCountData.doubleByteCount =
+ characterCount.doubleByteCount as number
+ characterCountData.tripleByteCount =
+ characterCount.tripleByteCount as number
+ characterCountData.quadByteCount = characterCount.quadByteCount as number
+ characterCountData.invalidBytes = characterCount.invalidBytes as number
+
+ setStatusMessage(
+ `Profiled bytes from ${startOffset} to ${startOffset + length}`
+ )
})
+ length = length < 0 ? fileMetricsState.computedSize : length
+
endOffset = startOffset + length
- requestSessionProfile(startOffset, length)
+ requestSessionProfile(startOffset, endOffset)
Review Comment:
The second argument to `requestSessionProfile` appears to be a *length* (previously `requestSessionProfile(startOffset, length)`), but it is now passed `endOffset` (start+length). This will request/profile too much data and can cause out-of-range requests/perf issues. Pass `length` as the second argument (or rename the function/parameters if it truly expects an end offset).
```suggestion
requestSessionProfile(startOffset, length)
```
##########
src/dataEditor/ui/svelteWebviewInitializer.ts:
##########
@@ -0,0 +1,283 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as vscode from 'vscode'
+import * as fs from 'fs'
+import { MessageResponseMap, PostMessageArgs } from 'ext_types'
+import { DisplayState } from './displayState'
+
+export interface DataEditorUI extends vscode.Disposable {
+ readonly displayState: DisplayState
+ dispose(): void
+ setTitle(title: string): void
+ getMsgId(): string
+ postMessage<K extends keyof MessageResponseMap>(
+ ...payload: PostMessageArgs<MessageResponseMap, K>
+ ): void
+ reveal: vscode.WebviewPanel['reveal']
+ onDidReceiveMessage(listener: (e: any) => void, thisArg?: any): void
+ onDidDispose(listener: (e: any) => void): void
+}
+class SvelteUIWebviewPanel implements DataEditorUI {
+ static readonly uiViewId: string = 'dataEditor'
+ readonly displayState: DisplayState = new DisplayState()
+
+ constructor(
+ private vscodePanel: vscode.WebviewPanel,
+ private messengerId: string = ''
+ ) {}
+ dispose() {
+ this.vscodePanel.dispose()
+ }
+ getMsgId() {
+ return this.messengerId
+ }
+ setMsgId(id: string) {
+ if (this.messengerId !== '')
+ throw 'Cannot reassign messenger id after initialization.'
+
+ this.messengerId = id
+ }
+ setTitle(title: string) {
+ this.vscodePanel.title = title
+ }
+
+ postMessage<K extends keyof MessageResponseMap>(
+ ...msg: PostMessageArgs<MessageResponseMap, K>
+ ) {
+ const [type, payload] = msg as [K, MessageResponseMap[K]]
+ this.vscodePanel.webview.postMessage({
+ command: type,
+ id: this.messengerId,
+ data: payload,
+ })
+ }
+
+ public onDidReceiveMessage(listener: (e: any) => void, thisArg?: any) {
+ this.vscodePanel.webview.onDidReceiveMessage(listener, thisArg)
+ }
+
+ public readonly reveal = () => {
+ this.vscodePanel.reveal()
+ }
+
+ public onDidDispose(listener: (e: any) => void) {
+ this.vscodePanel.onDidDispose(listener)
+ }
+}
+
+export type SveltePanelArgs = {
+ title: string
+ column: vscode.ViewColumn
+ uiMsgId: string
+}
+
+interface SvelteInitializerState {
+ isContextValid: boolean
+ getExtensionCtx(): vscode.ExtensionContext
+ getSvelteWebviewInitializer(): SvelteWebviewInitializer
+}
+const InvalidInitializerState: SvelteInitializerState = {
+ isContextValid: false,
+ getSvelteWebviewInitializer: () => {
+ throw 'SvelteWebviewInitializer is in an invalid state'
+ },
+ getExtensionCtx: function (): vscode.ExtensionContext {
+ throw 'SvelteWebviewInitializer is in an invalid state'
+ },
+}
+
+const ValidInitializerState: SvelteInitializerState = {
+ isContextValid: true,
+ getSvelteWebviewInitializer: () => {
+ return SvelteWebviewMgr
+ },
+ getExtensionCtx: () => {
+ return ExtensionContextRef
+ },
+}
+
+const UI_MSG_ID_MAX_LEN = 32
+
+class SvelteWebviewInitializer {
+ static state: SvelteInitializerState = InvalidInitializerState
+ constructor() {}
+ formatMsgId(id: string, isDFDLDebugAttached: boolean = false) {
+ let idStr = id.substring(0, UI_MSG_ID_MAX_LEN)
+ idStr.replaceAll(' ', '_')
Review Comment:
`String.prototype.replaceAll` returns a new string; it does not mutate. As written, spaces are not replaced, so message IDs can still contain spaces. Assign the result back (e.g. `idStr = idStr.replaceAll(' ', '_')`).
```suggestion
idStr = idStr.replaceAll(' ', '_')
```
##########
src/dataEditor/ui/svelteWebviewInitializer.ts:
##########
@@ -0,0 +1,283 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as vscode from 'vscode'
+import * as fs from 'fs'
+import { MessageResponseMap, PostMessageArgs } from 'ext_types'
+import { DisplayState } from './displayState'
+
+export interface DataEditorUI extends vscode.Disposable {
+ readonly displayState: DisplayState
+ dispose(): void
+ setTitle(title: string): void
+ getMsgId(): string
+ postMessage<K extends keyof MessageResponseMap>(
+ ...payload: PostMessageArgs<MessageResponseMap, K>
+ ): void
+ reveal: vscode.WebviewPanel['reveal']
+ onDidReceiveMessage(listener: (e: any) => void, thisArg?: any): void
+ onDidDispose(listener: (e: any) => void): void
+}
+class SvelteUIWebviewPanel implements DataEditorUI {
+ static readonly uiViewId: string = 'dataEditor'
+ readonly displayState: DisplayState = new DisplayState()
+
+ constructor(
+ private vscodePanel: vscode.WebviewPanel,
+ private messengerId: string = ''
+ ) {}
+ dispose() {
+ this.vscodePanel.dispose()
+ }
+ getMsgId() {
+ return this.messengerId
+ }
+ setMsgId(id: string) {
+ if (this.messengerId !== '')
+ throw 'Cannot reassign messenger id after initialization.'
+
+ this.messengerId = id
+ }
+ setTitle(title: string) {
+ this.vscodePanel.title = title
+ }
+
+ postMessage<K extends keyof MessageResponseMap>(
+ ...msg: PostMessageArgs<MessageResponseMap, K>
+ ) {
+ const [type, payload] = msg as [K, MessageResponseMap[K]]
+ this.vscodePanel.webview.postMessage({
+ command: type,
+ id: this.messengerId,
+ data: payload,
+ })
+ }
+
+ public onDidReceiveMessage(listener: (e: any) => void, thisArg?: any) {
+ this.vscodePanel.webview.onDidReceiveMessage(listener, thisArg)
+ }
+
+ public readonly reveal = () => {
+ this.vscodePanel.reveal()
+ }
+
+ public onDidDispose(listener: (e: any) => void) {
+ this.vscodePanel.onDidDispose(listener)
+ }
+}
+
+export type SveltePanelArgs = {
+ title: string
+ column: vscode.ViewColumn
+ uiMsgId: string
+}
+
+interface SvelteInitializerState {
+ isContextValid: boolean
+ getExtensionCtx(): vscode.ExtensionContext
+ getSvelteWebviewInitializer(): SvelteWebviewInitializer
+}
+const InvalidInitializerState: SvelteInitializerState = {
+ isContextValid: false,
+ getSvelteWebviewInitializer: () => {
+ throw 'SvelteWebviewInitializer is in an invalid state'
+ },
+ getExtensionCtx: function (): vscode.ExtensionContext {
+ throw 'SvelteWebviewInitializer is in an invalid state'
+ },
+}
+
+const ValidInitializerState: SvelteInitializerState = {
+ isContextValid: true,
+ getSvelteWebviewInitializer: () => {
+ return SvelteWebviewMgr
+ },
+ getExtensionCtx: () => {
+ return ExtensionContextRef
+ },
+}
+
+const UI_MSG_ID_MAX_LEN = 32
+
+class SvelteWebviewInitializer {
+ static state: SvelteInitializerState = InvalidInitializerState
+ constructor() {}
+ formatMsgId(id: string, isDFDLDebugAttached: boolean = false) {
+ let idStr = id.substring(0, UI_MSG_ID_MAX_LEN)
+ idStr.replaceAll(' ', '_')
+ return isDFDLDebugAttached ? 'dfdl-' + idStr : idStr
+ }
+ createSveltePanel(args: SveltePanelArgs): SvelteUIWebviewPanel {
+ const opts = this.getWebviewOptions()
+ const ret = vscode.window.createWebviewPanel(
+ SvelteUIWebviewPanel.uiViewId,
+ args.title,
+ args.column,
+ {
+ ...opts,
+ }
+ )
+ this.setHtmlContent(ret, args.uiMsgId)
+
+ return new SvelteUIWebviewPanel(ret, args.uiMsgId)
+ }
+
+ // get the HTML content for the webview
+ private setHtmlContent(vsPanel: vscode.WebviewPanel, msgId: string) {
+ let vsWebview = vsPanel.webview
+ const nonce = this.getNonce()
+
+ const scriptUri = this.getResourceUri('js', ExtensionContextRef, (uri) => {
+ return vsWebview.asWebviewUri(uri)
+ })
+ const stylesUri = this.getResourceUri('css', ExtensionContextRef, (uri) => {
+ return vsWebview.asWebviewUri(uri)
+ })
+ const indexPath = this.getResourceUri('index', ExtensionContextRef)
+ let indexHTML = this.injectNonce(
+ this.getIndexHTML(ExtensionContextRef),
+ vsWebview,
+ nonce,
+ scriptUri
+ )!
+ indexHTML = fs
+ .readFileSync(indexPath!.fsPath, 'utf-8')
+ .replace(/src="\.\/index.js"/, `src="${scriptUri.toString()}"`)
+ .replace(/href="\.\/style.css"/, `href="${stylesUri.toString()}"`)
+ .replaceAll(/nonce="__nonce__"/g, `nonce="${nonce}""`)
Review Comment:
Two issues here: (1) `injectNonce(...)` is immediately overwritten by re-reading `index.html`, so the CSP `<meta>` injection is dropped (weakening webview CSP). (2) the nonce replacement injects an extra quote (`nonce=\"${nonce}\"\"`), producing invalid HTML attributes. Use a single HTML source string (read once, then inject CSP + replace URIs + replace nonce with `nonce=\"${nonce}\"`) so CSP is actually applied and attributes are valid.
```suggestion
let indexHTML = this.injectNonce(
this.getIndexHTML(ExtensionContextRef),
vsWebview,
nonce,
scriptUri
)!
indexHTML = indexHTML
.replace(/src="\.\/index.js"/, `src="${scriptUri.toString()}"`)
.replace(/href="\.\/style.css"/, `href="${stylesUri.toString()}"`)
.replaceAll(/nonce="__nonce__"/g, `nonce="${nonce}"`)
```
##########
src/ext_types/messageContent.ts:
##########
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import {
+ ApplyChangesRequest,
+ ChangesInfoResponse,
+ CountResponse,
+ DFDLDataBytePos,
+ EditedDataRequest,
+ EditedDataResponse,
+ EditorOnChangeRequest,
+ EditorOnChangeResponse,
+ FileInfoResponse,
+ HeartbeatResponse,
+ NotificationRequest,
+ ProfileRequest,
+ ProfileResponse,
+ ReplaceRequest,
+ ReplaceResponse,
+ SaveAsResponse,
+ SaveResponse,
+ SaveSegmentRequest,
+ ScrollViewportRequest,
+ SearchRequest,
+ SearchResponse,
+ SetUIThemeResponse,
+ ViewportRefreshResponse,
+} from './messages'
+import { EditorRequestIds, isEditorMessageId } from './messageIds'
+
+type MessageCommands =
+ | 'clearChanges'
+ | 'applyChanges'
+ | 'editorOnChange'
+ | 'fileInfo'
+ | 'heartbeat'
+ | 'profile'
+ | 'redoChange'
+ | 'replaceResults'
+ | 'requestEditedData'
+ | 'save'
+ | 'saveAs'
+ | 'saveSegment'
+ | 'scrollViewport'
+ | 'search'
+ | 'replace'
+ | 'setUITheme'
+ | 'showMessage'
+ | 'undoChange'
+ | 'viewportRefresh'
+
+type CommandMap = {
+ [K in MessageCommands]: any
+}
+export interface MessageRequestMap extends CommandMap {
+ counts: never
+ clearChanges: never
+ applyChanges: ApplyChangesRequest
+ editorOnChange: EditorOnChangeRequest
+ fileInfo: never
+ heartbeat: never
+ profile: ProfileRequest
+ redoChange: never
+ replaceResults: never
+ requestEditedData: EditedDataRequest
+ save: never
+ saveAs: never
+ saveSegment: SaveSegmentRequest
+ scrollViewport: ScrollViewportRequest
+ search: SearchRequest
+ replace: ReplaceRequest
+ undoChange: never
+ viewportRefresh: never
+ showMessage: NotificationRequest
+}
+
+export interface MessageResponseMap extends CommandMap {
+ clearChanges: void
+ applyChanges: ChangesInfoResponse
+ editorOnChange: EditorOnChangeResponse
+ fileInfo: FileInfoResponse
+ counts: CountResponse
+ profile: ProfileResponse
+ redoChange: void
+ replaceResults: ReplaceResponse
+ requestEditedData: EditedDataResponse
+ save: SaveResponse
+ saveAs: SaveAsResponse
+ saveSegment: void
+ scrollViewport: void
+ search: SearchResponse
+ replace: ReplaceResponse
+ undoChange: void
+ viewportRefresh: ViewportRefreshResponse
+ showMessage: void
+ setUITheme: SetUIThemeResponse
+ heartbeat: HeartbeatResponse
+ bytePos1b: DFDLDataBytePos
+}
+
+export type ExtensionMessageKeys =
+ | 'showMessage'
+ | 'setUITheme'
+ | 'editorOnChange'
+
+export type DataEditorMessageKeys = Exclude<
+ MessageCommands,
+ ExtensionMessageKeys
+>
+export type DataEditorMessageRequests = Pick<
+ MessageRequestMap,
+ DataEditorMessageKeys
+>
+export type DataEditorMessageResponses = Pick<
+ MessageResponseMap,
+ DataEditorMessageKeys
+>
+/**
+ * Key indexable interface to templated type inference of available messages sent between
+ * the components of the DFDL VSCode extension.
+ */
+export type ExtensionMessageRequests = Pick<
+ MessageRequestMap,
+ ExtensionMessageKeys
+>
+
+export interface DFDLSessionMessageResponses extends MessageResponseMap {
+ bytesPos1b: DFDLDataBytePos
Review Comment:
The response id elsewhere is `bytePos1b` (singular), but this interface defines `bytesPos1b` (plural). This typo will break type-safe lookups and can lead to mismatched message handling. Rename to `bytePos1b` to match the actual message id.
```suggestion
bytePos1b: DFDLDataBytePos
```
##########
src/dataEditor/dataEditorClient.ts:
##########
@@ -184,50 +187,56 @@ export class DataEditorClient implements vscode.Disposable {
this.disposables[i].dispose()
}
- show(): void {
+ public show(): void {
this.panel.reveal()
}
+
public static async open(
context: vscode.ExtensionContext,
- view: string,
configVars: editor_config.IConfig,
fileToEdit: string = ''
): Promise<DataEditorClient | undefined> {
- const title = !fileToEdit ? 'Data Editor' : path.basename(fileToEdit)
+ startSvelteWebviewInitializer(context)
+ const title = !fileToEdit
+ ? 'Data Editor' + (OPEN_EDITORS.size + 1).toString()
+ : path.basename(fileToEdit)
const column =
- fileToEdit !== '' ? vscode.ViewColumn.Two : vscode.ViewColumn.Active
-
- const panel = vscode.window.createWebviewPanel(view, title, column, {
- enableScripts: true,
- retainContextWhenHidden: true,
- })
-
- const editor = new DataEditorClient(
- context,
- view,
+ fileToEdit === '' ? vscode.ViewColumn.Two : vscode.ViewColumn.Active
+ const uiMsgId = getSvelteWebviewInitializer().formatMsgId(
title,
- configVars,
- fileToEdit,
- panel
+ isDFDLDebugSessionActive(fileToEdit)
)
+ const ui = getSvelteWebviewInitializer().createSveltePanel({
+ title,
+ column,
+ uiMsgId,
+ })
+ ui.displayState.onColorThemeChanged((kind) => {
+ ui.postMessage('setUITheme', kind)
+ })
+ const editor = new DataEditorClient(context, configVars, fileToEdit, ui)
+
await editor.initialize()
- panel.onDidDispose(async () => {
+ ui.reveal()
+
+ ui.onDidDispose(async () => {
const pathKey = path.resolve(editor.fileToEdit).toLowerCase()
OPEN_EDITORS.delete(pathKey)
Review Comment:
This code deletes from `OPEN_EDITORS` using a normalized `pathKey`, but `isActive()` later checks `OPEN_EDITORS.get(this.fileToEdit)`. If the map is keyed by normalized paths (as this deletion strongly implies), `isActive()` will return false even when the editor is open (and your updated test now depends on `isActive()`). Make `isActive()` use the same key normalization strategy as all `OPEN_EDITORS` accesses.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3141172168
##########
src/svelte/src/components/ServerMetrics/ServerMetrics.svelte:
##########
@@ -66,43 +68,49 @@ limitations under the License.
? `${seconds} second`
: `${seconds} seconds`
Review Comment:
`prettyPrintUptime()` has an operator-precedence bug: `return uptimeString + (seconds === 1) ? ... : ...` will evaluate `uptimeString + (seconds === 1)` first, producing a truthy string and always selecting the `?` branch. Wrap the ternary in parentheses so the suffix is chosen correctly.
##########
src/svelte/svelte.config.mjs:
##########
@@ -25,7 +25,9 @@ const config = {
sveltePreprocess({ sourceMap: true, typescript: true }),
],
compilerOptions: {
+ runes: false,
css: 'external',
+ rootDir: '.',
},
Review Comment:
`compilerOptions.runes` is set to `false`, but the UI code in this PR uses Svelte 5 runes extensively (e.g., `$state`, `$derived`). Disabling runes here will prevent those files from compiling/working. Set `runes: true` (or remove this override if the project relies on the default) so the new rune-based stores/components build correctly.
##########
src/svelte/src/components/Header/fieldsets/Settings.svelte:
##########
@@ -28,30 +28,45 @@ limitations under the License.
import FlexContainer from '../../layouts/FlexContainer.svelte'
import { UIThemeCSSClass } from '../../../utilities/colorScheme'
import ViewportVisibilityIcon from '../../Icons/ViewportVisibilityIcon.svelte'
- import { MessageCommand } from '../../../utilities/message'
+ import { vscode } from '../../../utilities/vscode'
+ const { addListener } = vscode.getMessenger(getUIMsgId())
/* DEBUG_ONLY_START */
import { getDebugVarContext } from '../../Debug/'
+ import { getUIMsgId } from 'stores/states.svelte'
+ import {
Review Comment:
`getUIMsgId()` is imported inside the `/* DEBUG_ONLY_START */ ... /* DEBUG_ONLY_END */` section, but it's used in production code when creating the messenger (`vscode.getMessenger(getUIMsgId())`). When DEBUG_ONLY code is stripped, this will leave `getUIMsgId` undefined and break the UI in non-debug builds. Move the `getUIMsgId` import (and any other runtime dependencies) outside the DEBUG_ONLY block, or move the messenger initialization inside the same block if it’s truly debug-only.
##########
src/svelte/vitest.config.mjs:
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { defineConfig } from 'vitest/config'
+import { fileURLToPath } from 'node:url'
+import { svelte } from '@sveltejs/vite-plugin-svelte'
+
+/// <reference types="vitest/config"/>
+export default defineConfig(({ mode }) => {
+ return {
+ root: fileURLToPath(new URL('./', import.meta.url)),
+ plugins: [svelte({ configFile: './svelte.config.mjs' })],
+ resolve: {
+ tsconfigPaths: true,
+ alias: {
+ '@': fileURLToPath(new URL('./src', import.meta.url)),
+ utilities: fileURLToPath(new URL('./src/utilities', import.meta.url)),
+ layout: fileURLToPath(
+ new URL('./src/components/layouts', import.meta.url)
+ ),
+ HTMLWrappers: fileURLToPath(
+ new URL('./src/components/html', import.meta.url)
+ ),
+ editor_components: fileURLToPath(
+ new URL('./src/components', import.meta.url)
+ ),
+ ext_types: fileURLToPath(new URL('../ext_types', import.meta.url)),
+ stores: fileURLToPath(new URL('./src/stores', import.meta.url)),
+ },
+ },
+ test: {
+ environment: 'jsdom',
+ include: ['./tests/**/*.svelte.test.ts'],
+ exclude: ['node_modules/**', 'out/**'],
+ typecheck: {
+ enabled: true,
+ tsconfig: './src/svelte/tsconfig.json',
+ include: ['./src/svelte/tests/**/*.svelte.test.ts'],
+ ignoreSourceErrors: true,
+ },
Review Comment:
In this vitest config, `root` is set to the `src/svelte` directory, but the `typecheck.tsconfig` and `typecheck.include` paths are still prefixed with `./src/svelte/...`, which likely resolves to a non-existent `src/svelte/src/svelte/...` path. Adjust these paths to be relative to the configured `root` (e.g., `./tsconfig.json` and `./tests/**/...`).
##########
src/ext_types/messageContent.ts:
##########
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import {
+ ApplyChangesRequest,
+ ChangesInfoResponse,
+ CountResponse,
+ DFDLDataBytePos,
+ EditedDataRequest,
+ EditedDataResponse,
+ EditorOnChangeRequest,
+ EditorOnChangeResponse,
+ FileInfoResponse,
+ HeartbeatResponse,
+ NotificationRequest,
+ ProfileRequest,
+ ProfileResponse,
+ ReplaceRequest,
+ ReplaceResponse,
+ SaveAsResponse,
+ SaveResponse,
+ SaveSegmentRequest,
+ ScrollViewportRequest,
+ SearchRequest,
+ SearchResponse,
+ SetUIThemeResponse,
+ ViewportRefreshResponse,
+} from './messages'
+import { EditorRequestIds, isEditorMessageId } from './messageIds'
+
+type MessageCommands =
+ | 'clearChanges'
+ | 'applyChanges'
+ | 'editorOnChange'
+ | 'fileInfo'
+ | 'heartbeat'
+ | 'profile'
+ | 'redoChange'
+ | 'replaceResults'
+ | 'requestEditedData'
+ | 'save'
+ | 'saveAs'
+ | 'saveSegment'
+ | 'scrollViewport'
+ | 'search'
+ | 'replace'
+ | 'setUITheme'
+ | 'showMessage'
+ | 'undoChange'
+ | 'viewportRefresh'
+
+type CommandMap = {
+ [K in MessageCommands]: any
+}
+export interface MessageRequestMap extends CommandMap {
+ counts: never
+ clearChanges: never
+ applyChanges: ApplyChangesRequest
+ editorOnChange: EditorOnChangeRequest
+ fileInfo: never
+ heartbeat: never
+ profile: ProfileRequest
+ redoChange: never
+ replaceResults: never
+ requestEditedData: EditedDataRequest
+ save: never
+ saveAs: never
+ saveSegment: SaveSegmentRequest
+ scrollViewport: ScrollViewportRequest
+ search: SearchRequest
+ replace: ReplaceRequest
+ undoChange: never
+ viewportRefresh: never
+ showMessage: NotificationRequest
+}
+
+export interface MessageResponseMap extends CommandMap {
+ clearChanges: void
+ applyChanges: ChangesInfoResponse
+ editorOnChange: EditorOnChangeResponse
+ fileInfo: FileInfoResponse
+ counts: CountResponse
+ profile: ProfileResponse
+ redoChange: void
+ replaceResults: ReplaceResponse
+ requestEditedData: EditedDataResponse
Review Comment:
`MessageResponseMap` uses `void` for no-payload responses (e.g., `clearChanges`, `redoChange`, etc.), but `PostMessageArgs` only treats `never` as “no payload”. This makes calls like `panel.postMessage('clearChanges')` (used in `dataEditorClient.ts`) fail type-checking because the signature expects a second argument. Consider using `never` for no-payload response entries, or update `PostMessageArgs` to treat `void | undefined` similarly to `never`.
##########
src/svelte/vite.config.mjs:
##########
@@ -1,81 +1,59 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
-import { defineConfig } from 'vite'
+// http://www.apache.org/licenses/LICENSE-2.0
+
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { defineConfig, mergeConfig } from 'vitest/config'
import { svelte } from '@sveltejs/vite-plugin-svelte'
-import path from 'path'
+import { fileURLToPath } from 'node:url'
import strip from 'rollup-plugin-strip-code'
-import { fileURLToPath } from 'url'
-import { loadEnvFile } from 'node:process'
import * as fs from 'fs'
+import path from 'path'
+
Review Comment:
`loadEnvFile(envFilePath)` is called in this config, but `loadEnvFile` is not imported anywhere (it was removed from the import list). This will throw at runtime when the Vite config is evaluated. Add the appropriate import (or replace with the intended env-loading mechanism).
##########
src/ext_types/messages.ts:
##########
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { ServerHeartbeat } from 'dataEditor/include/server/heartbeat/HeartBeatInfo'
+import { EditByteModes } from './formattypes'
+import { ColorThemeKind } from 'vscode'
Review Comment:
This shared `ext_types` module is re-exported into the Svelte/webview bundle, but it currently has runtime imports from extension-only modules (`vscode`, and `dataEditor/...`). Since these are only used for typing (`ColorThemeKind`, `ServerHeartbeat`), they should be `import type` to avoid bundling/trying to resolve `vscode` in the webview build. Otherwise the UI bundle can fail to build or crash at runtime due to missing modules.
```suggestion
import type { ServerHeartbeat } from 'dataEditor/include/server/heartbeat/HeartBeatInfo'
import { EditByteModes } from './formattypes'
import type { ColorThemeKind } from 'vscode'
```
##########
src/svelte/src/components/ServerMetrics/ServerMetrics.svelte:
##########
@@ -66,43 +68,49 @@ limitations under the License.
? `${seconds} second`
: `${seconds} seconds`
}
-
- window.addEventListener('message', (msg) => {
- switch (msg.data.command) {
- case MessageCommand.heartbeat:
- heartbeat.latency = msg.data.data.latency
- heartbeat.serverCpuLoadAverage = msg.data.data.serverCpuLoadAverage
- heartbeat.serverTimestamp = msg.data.data.serverTimestamp
- heartbeat.serverUptime = msg.data.data.serverUptime
- heartbeat.serverUsedMemory = msg.data.data.serverUsedMemory
- heartbeat.sessionCount = msg.data.data.sessionCount
- heartbeat.omegaEditPort = msg.data.data.serverInfo.omegaEditPort
- heartbeat.serverVersion = msg.data.data.serverInfo.serverVersion
- heartbeat.serverHostname = msg.data.data.serverInfo.serverHostname
- heartbeat.serverProcessId = msg.data.data.serverInfo.serverProcessId
- heartbeat.jvmVersion = msg.data.data.serverInfo.jvmVersion
- heartbeat.jvmVendor = msg.data.data.serverInfo.jvmVendor
- heartbeat.jvmPath = msg.data.data.serverInfo.jvmPath
- heartbeat.availableProcessors =
- msg.data.data.serverInfo.availableProcessors
-
- // set the serverTimestamp to 0 after 5 seconds of no heartbeat to indicate that no heartbeat has been received
- clearTimeout(timerId)
- timerId = setTimeout(() => {
- heartbeat.serverTimestamp = 0
- }, 5000)
- break
- }
+ addListener('heartbeat', (data) => {
+ const {
+ latency,
+ port,
+ serverCpuCount,
+ serverCpuLoadAverage,
+ serverTimestamp,
+ serverUptime,
+ serverUsedMemory,
+ sessionCount,
+ serverInfo,
+ } = data
+ serverInfo
+ heartbeat.latency = latency
+ heartbeat.port = port
+ heartbeat.serverCpuLoadAverage = serverCpuLoadAverage
+ heartbeat.availableProcessors = serverCpuCount
+ heartbeat.serverTimestamp = serverTimestamp
+ heartbeat.serverUptime = serverUptime
+ heartbeat.serverUsedMemory = serverUsedMemory
+ heartbeat.sessionCount = sessionCount
+ heartbeat.availableProcessors = serverInfo.availableProcessors
+ heartbeat.serverHostname = serverInfo.serverHostname
Review Comment:
In the heartbeat listener, `availableProcessors` is assigned from `serverCpuCount` and then immediately overwritten by `serverInfo.availableProcessors`, and there’s also a stray `serverInfo` expression statement. This is likely unintended and can lead to confusing/incorrect displayed values. Keep only the intended source and remove the no-op statement.
##########
src/svelte/tsconfig.json:
##########
@@ -40,7 +44,15 @@
"esModuleInterop": true,
"verbatimModuleSyntax": true
},
- "include": ["src/**/*.ts", "src/**/*.svelte", "src/tests/**/*.test.ts"],
+ "include": [
+ "tests/**/*.svelte.test.ts",
+ "src/**/*.ts",
+ "src/**/*.svelte",
+ "src/tests/**/*.test.ts",
+ "../types/**/*.d.ts",
+ "src/**/*.svelte.test.ts",
+ "src/svelte/tests/**.*.svelte.test.ts"
+ ],
Review Comment:
The include glob `"src/svelte/tests/**.*.svelte.test.ts"` looks malformed (`**.*.`) and likely won’t match anything. This can cause tests/typechecking to be skipped unexpectedly. Replace it with the intended glob (e.g., `src/svelte/tests/**/*.svelte.test.ts`, or just `tests/**/*.svelte.test.ts` given this config’s baseUrl/root).
##########
src/svelte/src/components/DataDisplays/CustomByteDisplay/DataLineFeed.svelte:
##########
@@ -71,13 +67,17 @@ limitations under the License.
import { bytesPerRow } from '../../../stores'
/* DEBUG_ONLY_START */
- import { addVarToDebug, getDebugVarContext } from '../../Debug'
+ import { getDebugVarContext } from '../../Debug'
+ import { vscode } from 'utilities/vscode'
+ import { getUIMsgId, isUIDebugAttached } from 'stores/states.svelte'
+ import { fileMetricsState } from 'editor_components/Header/fieldsets/FileMetrics.svelte.ts'
const debugVarsCtx = getDebugVarContext()
debugVarsCtx.add(
{ id: 'bytes / row', valueStr: () => $bytesPerRow.toString() },
{ id: 'feed line top', valueStr: () => $dataFeedLineTop.toString() }
)
/* DEBUG_ONLY_END */
+ const { postMessage, addListener } = vscode.getMessenger(getUIMsgId())
export let awaitViewportSeek: boolean
Review Comment:
Messenger initialization (`const { postMessage, addListener } = vscode.getMessenger(getUIMsgId())`) occurs after the DEBUG_ONLY section, but its dependencies (`vscode`/`getUIMsgId`) are currently imported only inside DEBUG_ONLY. This will compile/run in debug builds but fail in production where strip-code removes those imports.
##########
src/svelte/src/utilities/vscode.ts:
##########
@@ -28,15 +48,45 @@ import type { WebviewApi } from 'vscode-webview'
*/
class VSCodeAPIWrapper {
private readonly vsCodeApi: WebviewApi<unknown> | undefined
-
constructor() {
// Check if the acquireVsCodeApi function exists in the current development
// context (i.e. VS Code development window or web browser)
if (typeof acquireVsCodeApi === 'function') {
this.vsCodeApi = acquireVsCodeApi()
}
}
+ getMessenger(id: string): VSMessenger {
+ return {
+ postMessage: <K extends keyof MessageRequestMap>(
+ ...args: PostMessageArgs<MessageRequestMap, K>
+ ) => {
+ let msg = this.createMessage(args)
+ msg.id = id
+ this._postMessage(msg)
+ },
+ addListener: <K extends keyof MessageResponseMap>(
+ type: K,
+ listener: (data: MessageResponseMap[K]) => void
+ ) => {
+ this._addListener(id, type, listener)
+ },
+ }
+ }
+ private _addListener<K extends keyof MessageResponseMap>(
+ id: string,
+ type: K,
+ listener: (data: MessageResponseMap[K]) => void
+ ) {
+ window.addEventListener(type, (winEvent: WindowEventMap[K]) => {
+ const eventType = winEvent.type
+ if (!isEditorResponseId(eventType)) return
+
+ const details = winEvent.detail
+ if (id !== details.id) return
+ listener(details.data as MessageResponseMap[K])
+ })
Review Comment:
`_addListener()` types the event as `WindowEventMap[K]` and accesses `winEvent.detail`, but `K` here is a message id (e.g., `'heartbeat'`) and is not a key of `WindowEventMap`. This is likely a TypeScript error and also mis-types the event payload (these are `CustomEvent`s). Use `CustomEvent<{ id: string; data: ... }>` (or `Event` + a cast) instead of `WindowEventMap[K]` so indexing and `.detail` are valid.
##########
src/svelte/src/components/DataDisplays/CustomByteDisplay/DataLineFeed.svelte:
##########
@@ -71,13 +67,17 @@ limitations under the License.
import { bytesPerRow } from '../../../stores'
/* DEBUG_ONLY_START */
- import { addVarToDebug, getDebugVarContext } from '../../Debug'
+ import { getDebugVarContext } from '../../Debug'
+ import { vscode } from 'utilities/vscode'
+ import { getUIMsgId, isUIDebugAttached } from 'stores/states.svelte'
+ import { fileMetricsState } from 'editor_components/Header/fieldsets/FileMetrics.svelte.ts'
const debugVarsCtx = getDebugVarContext()
Review Comment:
`vscode`, `getUIMsgId`, and `fileMetricsState` are imported inside the `/* DEBUG_ONLY_START */ ... /* DEBUG_ONLY_END */` block, but they are referenced later outside that block (e.g., `vscode.getMessenger(getUIMsgId())` and `fileMetricsState.computedSize`). When DEBUG_ONLY code is stripped for production builds, these identifiers will be undefined, causing runtime failures. Move these imports outside the DEBUG_ONLY block (and keep only debug-variable wiring inside it).
##########
src/svelte/src/components/Header/fieldsets/FileMetrics.svelte:
##########
@@ -17,38 +17,39 @@ limitations under the License.
<script lang="ts">
import Button from '../../Inputs/Buttons/Button.svelte'
import FlexContainer from '../../layouts/FlexContainer.svelte'
- import { MessageCommand } from '../../../utilities/message'
- import { vscode } from '../../../utilities/vscode'
- import { saveable, fileMetrics, replaceQuery } from '../../../stores'
import { createEventDispatcher } from 'svelte'
import SidePanel from '../../layouts/SidePanel.svelte'
import ByteFrequencyGraph from '../../DataMetrics/DataMetrics.svelte'
- import { viewport } from '../../../stores'
import { humanReadableByteLength } from '../../../utilities/display'
import { DATA_PROFILE_MAX_LENGTH } from '../../../stores/configuration'
import Tooltip from '../../layouts/Tooltip.svelte'
import ISO6391 from 'iso-639-1'
+ import { getUIMsgId } from 'stores/states.svelte'
+ import { vscode } from 'utilities/vscode'
+ import {
+ canRedo,
+ canRevert,
+ canUndo,
+ fileMetricsState,
+ saveable,
+ } from './FileMetrics.svelte.ts'
+
const eventDispatcher = createEventDispatcher()
+ const { postMessage, addListener } = vscode.getMessenger(getUIMsgId())
+
let displayOpts = false
let isProfilerOpen = false
- let canUndo: boolean
- let canRedo: boolean
- let canRevert: boolean
let startOffset: number = 0
let length: number = 0
Review Comment:
`startOffset` and `length` are initialized to `0` and no longer updated anywhere, but they’re used to drive the Data Profiler (`<ByteFrequencyGraph {startOffset} length={Math.min(length, DATA_PROFILE_MAX_LENGTH)} />`). As-is, opening the profiler will request a 0-length profile from offset 0, which likely breaks/produces empty results. Reintroduce the logic that sets `length` (and potentially `startOffset`) based on the current file/viewport/selection, or provide sensible defaults (e.g., profile the first `DATA_PROFILE_MAX_LENGTH` bytes).
```suggestion
$: profileLengthSource = Math.max(
fileMetricsState.computedSize,
fileMetricsState.diskSize,
)
$: startOffset = 0
$: length =
profileLengthSource > 0
? Math.min(profileLengthSource, DATA_PROFILE_MAX_LENGTH)
: DATA_PROFILE_MAX_LENGTH
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4219281762
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#issuecomment-4482995064
I am going to close this PR to rework the conflicting portions w/ the OE version 2 upgrade. I will reopen it when I get it into shape.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls closed pull request #1647: Restructure data editor messaging with Message Registry
URL: https://github.com/apache/daffodil-vscode/pull/1647
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Restructure data editor messaging with Message Registry [daffodil-vscode]
Posted by "stricklandrbls (via GitHub)" <gi...@apache.org>.
stricklandrbls commented on code in PR #1647:
URL: https://github.com/apache/daffodil-vscode/pull/1647#discussion_r3245039773
##########
build/package/LICENSE:
##########
@@ -2202,6 +2202,22 @@ conditions of the following licenses.
under the terms of the GNU Lesser General Public License version 2.1
as published by the Free Software Foundation.
+- 'lightningcss' in extension/dist/ext/extension.js
+- 'lightningcss' in node_modules/vite
+ This product bundles 'lightningcss' from the above files.
+ These files are available under the MPL 2.0 license:
+
+ 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.
Review Comment:
@shanedell - If you have any insight as well I'd appreciate it.
##########
build/package/LICENSE:
##########
@@ -2202,6 +2202,22 @@ conditions of the following licenses.
under the terms of the GNU Lesser General Public License version 2.1
as published by the Free Software Foundation.
+- 'lightningcss' in extension/dist/ext/extension.js
+- 'lightningcss' in node_modules/vite
+ This product bundles 'lightningcss' from the above files.
+ These files are available under the MPL 2.0 license:
+
+ 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.
Review Comment:
@shanedell - If you have any insight as well I'd appreciate it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org