Copilot commented on code in PR #1726: URL: https://github.com/apache/daffodil-vscode/pull/1726#discussion_r3439922936
########## src/dataEditor/omegaEditDataEditor.ts: ########## @@ -0,0 +1,325 @@ +/* + * 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 path from 'path' +import * as vscode from 'vscode' +import { + DaffodilData, + dataEvent, + extractDaffodilEvent, +} from '../daffodilDebugger/daffodil' +import { substituteVSCodeEnvVariables } from '../utils' + +export const DATA_EDITOR_COMMAND = 'extension.data.edit' +export const OMEGA_EDIT_EXTENSION_ID = 'ctc-oss.omega-edit-data-editor' +export const OMEGA_EDIT_EXTENSION_API_VERSION = 1 +export const DAFFODIL_CURRENT_DATA_HIGHLIGHT_ID = + 'apache-daffodil.current-data-byte' + +export type OmegaEditExternalHighlightKind = + | 'current' + | 'parsed' + | 'error' + | 'warning' + | 'breakpoint' + | 'secondary' + +export interface OmegaEditExternalHighlight { + id: string + offset: number + length: number + kind: OmegaEditExternalHighlightKind + label: string + source?: string +} + +export interface OmegaEditEditorState { + uri: string + filePath: string + fileSize: number + externalHighlights: OmegaEditExternalHighlight[] +} + +export interface OmegaEditEditorSelector { + uri?: vscode.Uri | string +} + +export interface OmegaEditOpenOptions { + offset?: number +} + +export interface OmegaEditExternalHighlightRequest + extends OmegaEditEditorSelector { + highlights: OmegaEditExternalHighlight[] + reveal?: boolean +} + +export interface OmegaEditExtensionApi { + readonly extensionId: typeof OMEGA_EDIT_EXTENSION_ID + readonly version: typeof OMEGA_EDIT_EXTENSION_API_VERSION + readonly onDidChangeEditorState: vscode.Event<OmegaEditEditorState> + open( + uri: vscode.Uri, + options?: OmegaEditOpenOptions + ): Promise<OmegaEditEditorState | undefined> + getEditorState( + options?: vscode.Uri | string | OmegaEditEditorSelector + ): OmegaEditEditorState | undefined + setExternalHighlights( + request: OmegaEditExternalHighlightRequest + ): Promise<OmegaEditEditorState | undefined> + clearExternalHighlights( + options?: vscode.Uri | string | OmegaEditEditorSelector + ): OmegaEditEditorState | undefined +} + +const openDataEditorUris = new Map<string, vscode.Uri>() +let omegaEditApiPromise: Promise<OmegaEditExtensionApi> | undefined + +export function activate(ctx: vscode.ExtensionContext): void { + ctx.subscriptions.push( + vscode.commands.registerCommand( + DATA_EDITOR_COMMAND, + async (fileToEdit: string | vscode.Uri = '') => + await openDataEditor(fileToEdit) + ), + vscode.debug.onDidReceiveDebugSessionCustomEvent((event) => { + void handleDaffodilDebugEvent(event) + }), + vscode.debug.onDidTerminateDebugSession(() => { + void clearDaffodilDataHighlights() + }) + ) +} + +export async function openDataEditor( + fileToEdit: string | vscode.Uri = '' +): Promise<OmegaEditEditorState | undefined> { + const dataUri = await resolveDataFileUri(fileToEdit) + if (!dataUri) return undefined + + const api = await getOmegaEditApiForUserAction() + if (!api) return undefined + + const state = await api.open(dataUri) + if (state) { + openDataEditorUris.set(dataUri.toString(), dataUri) + } + return state +} + +export async function resolveDataFileUri( + fileToEdit: string | vscode.Uri = '' +): Promise<vscode.Uri | undefined> { + if (fileToEdit instanceof vscode.Uri) { + return fileToEdit + } + + const dataPath = fileToEdit.trim() + if (dataPath.length === 0) { + const fileUri = await vscode.window.showOpenDialog({ + canSelectMany: false, + openLabel: 'Select', + canSelectFiles: true, + canSelectFolders: false, + title: 'Select Data File', + }) + + if (!fileUri || !fileUri[0]) { + vscode.window.showInformationMessage( + 'Data Editor file opening cancelled.' + ) + return undefined + } + + return fileUri[0] + } + + const uri = dataPathLooksLikeUri(dataPath) + ? vscode.Uri.parse(dataPath) + : vscode.Uri.file(resolveDataPath(dataPath)) + + try { + const stat = await vscode.workspace.fs.stat(uri) + if ((stat.type & vscode.FileType.File) === 0) { + vscode.window.showErrorMessage( + `Data Editor can only open files: ${uri.fsPath || uri.toString()}` + ) + return undefined + } + } catch { + vscode.window.showErrorMessage( + `Data Editor file does not exist: ${uri.fsPath || uri.toString()}` + ) + return undefined + } + + return uri +} + +export function buildDaffodilDataHighlight( + bytePos1b: number, + fileSize: number +): OmegaEditExternalHighlight | undefined { + if ( + !Number.isFinite(bytePos1b) || + !Number.isInteger(bytePos1b) || + bytePos1b < 1 || + !Number.isFinite(fileSize) || + fileSize < 1 + ) { + return undefined + } + + const offset = Math.min(bytePos1b - 1, fileSize - 1) + return { + id: DAFFODIL_CURRENT_DATA_HIGHLIGHT_ID, + offset, + length: 1, + kind: 'current', + label: `Daffodil parser byte ${bytePos1b}`, + source: 'Apache Daffodil', + } +} + +async function handleDaffodilDebugEvent( + event: vscode.DebugSessionCustomEvent +): Promise<void> { + if (openDataEditorUris.size === 0) return + + const daffodilEvent = extractDaffodilEvent(event) + if (daffodilEvent?.event !== dataEvent) return + + const body = daffodilEvent.body as DaffodilData + let api: OmegaEditExtensionApi + try { + api = await getOmegaEditApi() + } catch { + return + } + + for (const [key, uri] of openDataEditorUris) { + const editorState = api.getEditorState(uri) + if (!editorState) { + openDataEditorUris.delete(key) + continue + } + + const highlight = buildDaffodilDataHighlight( + body.bytePos1b, + editorState.fileSize + ) + if (!highlight) continue + + await api.setExternalHighlights({ + uri, + highlights: [highlight], + reveal: true, + }) + } +} + +async function clearDaffodilDataHighlights(): Promise<void> { + if (!omegaEditApiPromise) return + + const api = await omegaEditApiPromise + for (const [key, uri] of openDataEditorUris) { + if (!api.getEditorState(uri)) { + openDataEditorUris.delete(key) + continue + } + + api.clearExternalHighlights(uri) + } +} Review Comment: `clearDaffodilDataHighlights()` awaits `omegaEditApiPromise` without handling rejection. If the dependency activation previously failed (e.g. extension missing), this will raise an unhandled promise rejection from the debug-termination handler. Wrap the await in try/catch (and consider clearing any stale state) to keep termination cleanup safe. ########## src/dataEditor/omegaEditDataEditor.ts: ########## @@ -0,0 +1,325 @@ +/* + * 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 path from 'path' +import * as vscode from 'vscode' +import { + DaffodilData, + dataEvent, + extractDaffodilEvent, +} from '../daffodilDebugger/daffodil' +import { substituteVSCodeEnvVariables } from '../utils' + +export const DATA_EDITOR_COMMAND = 'extension.data.edit' +export const OMEGA_EDIT_EXTENSION_ID = 'ctc-oss.omega-edit-data-editor' +export const OMEGA_EDIT_EXTENSION_API_VERSION = 1 +export const DAFFODIL_CURRENT_DATA_HIGHLIGHT_ID = + 'apache-daffodil.current-data-byte' + +export type OmegaEditExternalHighlightKind = + | 'current' + | 'parsed' + | 'error' + | 'warning' + | 'breakpoint' + | 'secondary' + +export interface OmegaEditExternalHighlight { + id: string + offset: number + length: number + kind: OmegaEditExternalHighlightKind + label: string + source?: string +} + +export interface OmegaEditEditorState { + uri: string + filePath: string + fileSize: number + externalHighlights: OmegaEditExternalHighlight[] +} + +export interface OmegaEditEditorSelector { + uri?: vscode.Uri | string +} + +export interface OmegaEditOpenOptions { + offset?: number +} + +export interface OmegaEditExternalHighlightRequest + extends OmegaEditEditorSelector { + highlights: OmegaEditExternalHighlight[] + reveal?: boolean +} + +export interface OmegaEditExtensionApi { + readonly extensionId: typeof OMEGA_EDIT_EXTENSION_ID + readonly version: typeof OMEGA_EDIT_EXTENSION_API_VERSION + readonly onDidChangeEditorState: vscode.Event<OmegaEditEditorState> + open( + uri: vscode.Uri, + options?: OmegaEditOpenOptions + ): Promise<OmegaEditEditorState | undefined> + getEditorState( + options?: vscode.Uri | string | OmegaEditEditorSelector + ): OmegaEditEditorState | undefined + setExternalHighlights( + request: OmegaEditExternalHighlightRequest + ): Promise<OmegaEditEditorState | undefined> + clearExternalHighlights( + options?: vscode.Uri | string | OmegaEditEditorSelector + ): OmegaEditEditorState | undefined +} + +const openDataEditorUris = new Map<string, vscode.Uri>() +let omegaEditApiPromise: Promise<OmegaEditExtensionApi> | undefined + +export function activate(ctx: vscode.ExtensionContext): void { + ctx.subscriptions.push( + vscode.commands.registerCommand( + DATA_EDITOR_COMMAND, + async (fileToEdit: string | vscode.Uri = '') => + await openDataEditor(fileToEdit) + ), + vscode.debug.onDidReceiveDebugSessionCustomEvent((event) => { + void handleDaffodilDebugEvent(event) + }), + vscode.debug.onDidTerminateDebugSession(() => { + void clearDaffodilDataHighlights() + }) + ) +} + +export async function openDataEditor( + fileToEdit: string | vscode.Uri = '' +): Promise<OmegaEditEditorState | undefined> { + const dataUri = await resolveDataFileUri(fileToEdit) + if (!dataUri) return undefined + + const api = await getOmegaEditApiForUserAction() + if (!api) return undefined + + const state = await api.open(dataUri) + if (state) { + openDataEditorUris.set(dataUri.toString(), dataUri) + } + return state +} + +export async function resolveDataFileUri( + fileToEdit: string | vscode.Uri = '' +): Promise<vscode.Uri | undefined> { + if (fileToEdit instanceof vscode.Uri) { + return fileToEdit + } + + const dataPath = fileToEdit.trim() + if (dataPath.length === 0) { + const fileUri = await vscode.window.showOpenDialog({ + canSelectMany: false, + openLabel: 'Select', + canSelectFiles: true, + canSelectFolders: false, + title: 'Select Data File', + }) + + if (!fileUri || !fileUri[0]) { + vscode.window.showInformationMessage( + 'Data Editor file opening cancelled.' + ) + return undefined + } + + return fileUri[0] + } + + const uri = dataPathLooksLikeUri(dataPath) + ? vscode.Uri.parse(dataPath) + : vscode.Uri.file(resolveDataPath(dataPath)) + + try { + const stat = await vscode.workspace.fs.stat(uri) + if ((stat.type & vscode.FileType.File) === 0) { + vscode.window.showErrorMessage( + `Data Editor can only open files: ${uri.fsPath || uri.toString()}` + ) + return undefined + } + } catch { + vscode.window.showErrorMessage( + `Data Editor file does not exist: ${uri.fsPath || uri.toString()}` + ) + return undefined + } + + return uri +} + +export function buildDaffodilDataHighlight( + bytePos1b: number, + fileSize: number +): OmegaEditExternalHighlight | undefined { + if ( + !Number.isFinite(bytePos1b) || + !Number.isInteger(bytePos1b) || + bytePos1b < 1 || + !Number.isFinite(fileSize) || + fileSize < 1 + ) { + return undefined + } + + const offset = Math.min(bytePos1b - 1, fileSize - 1) + return { + id: DAFFODIL_CURRENT_DATA_HIGHLIGHT_ID, + offset, + length: 1, + kind: 'current', + label: `Daffodil parser byte ${bytePos1b}`, + source: 'Apache Daffodil', + } +} + +async function handleDaffodilDebugEvent( + event: vscode.DebugSessionCustomEvent +): Promise<void> { + if (openDataEditorUris.size === 0) return + + const daffodilEvent = extractDaffodilEvent(event) + if (daffodilEvent?.event !== dataEvent) return + + const body = daffodilEvent.body as DaffodilData + let api: OmegaEditExtensionApi + try { + api = await getOmegaEditApi() + } catch { + return + } + + for (const [key, uri] of openDataEditorUris) { + const editorState = api.getEditorState(uri) + if (!editorState) { + openDataEditorUris.delete(key) + continue + } + + const highlight = buildDaffodilDataHighlight( + body.bytePos1b, + editorState.fileSize + ) + if (!highlight) continue + + await api.setExternalHighlights({ + uri, + highlights: [highlight], + reveal: true, + }) + } +} + +async function clearDaffodilDataHighlights(): Promise<void> { + if (!omegaEditApiPromise) return + + const api = await omegaEditApiPromise + for (const [key, uri] of openDataEditorUris) { + if (!api.getEditorState(uri)) { + openDataEditorUris.delete(key) + continue + } + + api.clearExternalHighlights(uri) + } +} + +async function getOmegaEditApiForUserAction(): Promise< + OmegaEditExtensionApi | undefined +> { + try { + return await getOmegaEditApi() + } catch (err) { + const message = err instanceof Error ? err.message : String(err) + vscode.window.showErrorMessage(message) + return undefined + } +} + +async function getOmegaEditApi(): Promise<OmegaEditExtensionApi> { + if (!omegaEditApiPromise) { + omegaEditApiPromise = activateOmegaEditApi() + } + + return await omegaEditApiPromise +} Review Comment: `getOmegaEditApi()` caches a rejected `omegaEditApiPromise`. If the OmegaEdit extension is missing/invalid once, subsequent calls will keep failing even after the dependency becomes available (until window reload). Reset the cached promise on failure so user actions can retry cleanly. ########## src/tests/suite/dataEditor.test.ts: ########## @@ -19,131 +19,65 @@ import * as assert from 'assert' import * as path from 'path' import * as vscode from 'vscode' import fs from 'fs' -import { TEST_SCHEMA } from './common' -import { after, before } from 'mocha' import { - createSimpleFileLogger, - getClientVersion, - getServerInfo, - setLogger, - startServer, - stopProcessUsingPID, -} from '@omega-edit/client' -import { - APP_DATA_PATH, + buildDaffodilDataHighlight, + DAFFODIL_CURRENT_DATA_HIGHLIGHT_ID, DATA_EDITOR_COMMAND, - DataEditorClient, - OMEGA_EDIT_HOST, - SERVER_START_TIMEOUT, -} from '../../dataEditor/dataEditorClient' -import { writeLogbackConfigFile } from '../../dataEditor/include/server/LogbackConfig' - -const testPort = 9009 // use a different port than the default for testing to avoid conflicts with running servers -const logLevel = 'debug' - -function getTestPidFile(serverPort: number) { - return path.join(APP_DATA_PATH, `test-serv-${serverPort}.pid`) -} + OMEGA_EDIT_EXTENSION_ID, + resolveDataFileUri, +} from '../../dataEditor' +import { PACKAGE_PATH, TEST_SCHEMA } from './common' suite('Data Editor Test Suite', () => { - // NOTE: Currently failing after glob update. Maybe add back in later? - // test('data edit command exists', async () => { - // assert.strictEqual( - // (await vscode.commands.getCommands()).includes(DATA_EDITOR_COMMAND), - // true - // ) - // }) - - suite('Editor Service', () => { - const pidFile = getTestPidFile(testPort) - const serverLogFile = path.join(APP_DATA_PATH, `test-serv-${testPort}.log`) - const logFile = path.join(APP_DATA_PATH, `test-dataEditor-${testPort}.log`) - let logConfigFile = '' - setLogger(createSimpleFileLogger(logFile, logLevel)) - - before(async () => { - logConfigFile = writeLogbackConfigFile( - path.join(APP_DATA_PATH, `test-serv-${testPort}.logconf.xml`), - serverLogFile, - logLevel - ) - const serverPid = (await Promise.race([ - startServer(testPort, OMEGA_EDIT_HOST, pidFile, { logConfigFile }), - new Promise((resolve, reject) => { - setTimeout( - () => - reject( - new Error( - `Server startup timed out after ${SERVER_START_TIMEOUT} seconds` - ) - ), - SERVER_START_TIMEOUT * 1000 - ) - }), - ])) as number | undefined - if (serverPid === undefined || serverPid <= 0) { - throw new Error('Server failed to start or PID is invalid') - } - }) + test('data edit command is contributed', () => { + const packageJson = JSON.parse(fs.readFileSync(PACKAGE_PATH, 'utf8')) + assert.strictEqual( + packageJson.contributes.commands.some( + (command) => command.command === DATA_EDITOR_COMMAND + ), + true + ) + }) - after(async () => { - assert.strictEqual(fs.existsSync(pidFile), true) - const pid = parseInt(fs.readFileSync(pidFile).toString()) - console.log(pid) - assert.strictEqual(await stopProcessUsingPID(pid), true) - if (fs.existsSync(logConfigFile)) { - fs.rmSync(logConfigFile, { force: true }) - } - }) + test('declares OmegaEdit data editor extension dependency', () => { + const packageJson = JSON.parse(fs.readFileSync(PACKAGE_PATH, 'utf8')) + assert.deepStrictEqual(packageJson.extensionDependencies, [ + OMEGA_EDIT_EXTENSION_ID, + ]) + }) Review Comment: The test currently requires `extensionDependencies` to equal exactly `[OMEGA_EDIT_EXTENSION_ID]`. That will fail if this extension ever adds another dependency (even though the OmegaEdit dependency is still correctly declared). Prefer asserting that the list contains the expected ID. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
