feat: add expression evaluation codelens#72
Conversation
…codex/implement-evaluation-result-display-in-lsp-server
There was a problem hiding this comment.
Pull Request Overview
This PR adds expression evaluation codelens functionality to provide interactive evaluation of expressions in POML files. The change enables users to see evaluation results directly in the editor through codelens commands.
- Tracks expression evaluations per token range to handle duplicate expressions separately
- Implements codelens provider to show "Evaluate" commands for each expression
- Records evaluation history for each expression with position-based tracking
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/poml/file.tsx | Adds expression token extraction, evaluation tracking, and recording mechanisms |
| packages/poml/tests/file.test.tsx | Adds tests to verify expression evaluation recording functionality |
| packages/poml-vscode/lsp/server.ts | Implements codelens provider and execute command handler for expression evaluation |
| packages/poml-vscode/tests/lsp.test.ts | Adds commented test case for expression evaluation command |
Comments suppressed due to low confidence (1)
packages/poml-vscode/lsp/server.ts:238
- Missing semicolon after the vscodeRange object assignment.
}
| export interface PomlToken { | ||
| type: 'element' | 'attribute' | 'attributeValue'; | ||
| type: 'element' | 'attribute' | 'attributeValue' | 'expression'; | ||
| range: Range; | ||
| element: string; | ||
| element?: string; | ||
| attribute?: string; // specified only if it's an attribute | ||
| value?: string; // specified only if it's an attribute value | ||
| expression?: string; // specified only if it's an expression | ||
| } |
There was a problem hiding this comment.
The PomlToken interface change makes the 'element' property optional while adding 'expression' as optional. This creates ambiguity about which properties should be present for different token types. Consider using a discriminated union type or separate interfaces for different token types to ensure type safety.
| if (range) { | ||
| this.recordEvaluation(range, errMessage); | ||
| } | ||
| this.reportError(errMessage, range, e); |
There was a problem hiding this comment.
The error handling logic has been restructured but the return statement is now outside the catch block. This means the function will always return an empty string even when evaluation succeeds, which breaks the expression evaluation functionality.
| type: 'expression', | ||
| range: { | ||
| start: range.start + match.index, | ||
| end: range.start + match.index + match[0].length - 1, |
There was a problem hiding this comment.
The end position calculation subtracts 1, but this may cause inconsistency with how ranges are handled elsewhere. The range should likely end at the exact position after the last character, not one position before it.
| end: range.start + match.index + match[0].length - 1, | |
| end: range.start + match.index + match[0].length, |
| type: 'expression', | ||
| range: { | ||
| start: pos.start + match.index, | ||
| end: pos.start + match.index + match[0].length - 1, |
There was a problem hiding this comment.
Same issue as above - the end position calculation subtracts 1, which may cause range inconsistency.
| end: pos.start + match.index + match[0].length - 1, | |
| end: pos.start + match.index + match[0].length, |
ultmaster
deleted the
v772xr-codex/implement-evaluation-result-display-in-lsp-server
branch
Summary
Testing
npm run build-webviewnpm run build-clinpm run lintnpm testpython -m pytest python/testsxvfb-run -a npm run compilexvfb-run -a npm run test-vscode(failed: module load errors, 8 tests failed)https://chatgpt.com/codex/tasks/task_e_688dea8b66d0832eb9d783cb03d63930