Support per-resource options for prompts#62
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes resource options management by moving contexts and stylesheets handling from individual webview panels to a shared SettingsManager. The change enables consistent per-resource configuration across the extension and properly honors these settings in test commands.
- Migrated resource options storage from POMLWebviewPanel to SettingsManager for centralized management
- Implemented automatic detection and loading of associated context and stylesheet files
- Updated test command to use the centralized resource options instead of empty arrays
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/poml-vscode/settings.ts | Added ResourceOptions interface and centralized storage/loading logic in SettingsManager |
| packages/poml-vscode/panel/panel.ts | Removed local resource options management and integrated with central SettingsManager |
| packages/poml-vscode/command/testCommand.ts | Updated to use centralized resource options instead of hardcoded empty arrays |
Comment on lines
+157
to
+167
| const addIfExists = (arr: string[], file: string) => { | ||
| if (fs.existsSync(file)) { | ||
| arr.push(file); | ||
| return true; | ||
| } | ||
| return false; | ||
| }; | ||
|
|
||
| let changed = false; | ||
| changed = addIfExists(contexts, `${base}.context.json`) || changed; | ||
| changed = addIfExists(stylesheets, `${base}.stylesheet.json`) || changed; |
There was a problem hiding this comment.
Using synchronous file operations (fs.existsSync) in the main thread can block the VS Code extension host. Consider using fs.promises.access() with proper error handling for better performance.
Suggested change
| const addIfExists = (arr: string[], file: string) => { | |
| if (fs.existsSync(file)) { | |
| arr.push(file); | |
| return true; | |
| } | |
| return false; | |
| }; | |
| let changed = false; | |
| changed = addIfExists(contexts, `${base}.context.json`) || changed; | |
| changed = addIfExists(stylesheets, `${base}.stylesheet.json`) || changed; | |
| const addIfExists = async (arr: string[], file: string): Promise<boolean> => { | |
| try { | |
| await fs.promises.access(file, fs.constants.F_OK); | |
| arr.push(file); | |
| return true; | |
| } catch { | |
| return false; | |
| } | |
| }; | |
| let changed = false; | |
| changed = await addIfExists(contexts, `${base}.context.json`) || changed; | |
| changed = await addIfExists(stylesheets, `${base}.stylesheet.json`) || changed; |
packages/poml-vscode/panel/panel.ts
Outdated
| // If we have changed resources, cancel any pending updates | ||
| const isResourceChange = resource.fsPath !== this._pomlUri.fsPath; | ||
| if (isResourceChange) { | ||
| if (isResourceChange || this.firstUpdate) { |
There was a problem hiding this comment.
The variable 'this.firstUpdate' is referenced but not defined in the visible code context. This will cause a runtime error if firstUpdate is not a class property.
ultmaster
deleted the
codex/refactor-resourceoptions-handling-in-settings
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SettingsManagerPOMLWebviewPanelTesting
npm run build-webviewnpm run build-clinpm run lintnpm testpython -m pytest python/testsxvfb-run -a npm run compilenpm run build-extensionxvfb-run -a npm run test-vscodehttps://chatgpt.com/codex/tasks/task_e_68886c324158832eb716cae990bb7119