refactor(vscode): enhance dx of working with extension#466
Conversation
|
📝 WalkthroughWalkthroughIntroduces a parser override system for heading inference (packages/parser), extends the VSCode extension with inspector/selection UI, override generation, decorations, and new commands, updates CI for VSCode builds, and refactors store factory options for base URL configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as VSCode Editor
participant Gen as Override Generator
participant Selection as Selection View
participant Clipboard as Clipboard
Editor->>Gen: startSelection(editor, fileName, version, start, end)
Gen->>Gen: Apply decorations (heading highlight, START/END markers)
Gen->>Selection: Update mode="selecting"
Selection->>Selection: Render selection UI with current lines
Editor->>Gen: onSelectionChange (user clicks line)
Gen->>Gen: Update start or end position based on clickMode
Gen->>Gen: Refresh decorations
Selection->>Selection: Re-render with updated range
loop User adjusts selection
Editor->>Gen: onSelectionChange
Gen->>Gen: setStart/setEnd with boundary normalization
Gen->>Selection: Update UI
end
Editor->>Selection: Click "Confirm" in selection UI
Selection->>Gen: confirm()
Gen->>Gen: Validate position (start <= end)
Gen->>Gen: Create ParserOverride JSON
Gen->>Clipboard: Copy JSON to clipboard
Gen->>Gen: cleanup() - reset state, clear decorations
Selection->>Selection: mode="idle"
sequenceDiagram
participant Filesystem as Filesystem
participant Loader as Loader
participant Registry as Registry
participant HeadingInference as Heading Inference
participant Content as Document Content
Loader->>Filesystem: Load overrides/{unicodeVersion}/{fileName}.json
Filesystem-->>Loader: ParserOverride JSON
Loader->>Registry: validateViaSchema + registerOverride
Registry->>Registry: Store in Map by unicodeVersion/fileName
Content->>HeadingInference: inferHeading(content, settings)
HeadingInference->>Registry: getOverride(fileName, unicodeVersion)
alt Override with explicit position
Registry-->>HeadingInference: {heading: {position: {start, end}}}
HeadingInference->>Content: Extract lines[start..end]
Content-->>HeadingInference: Exact heading lines
else Override with heading settings
Registry-->>HeadingInference: {heading: {allowEmptyLines, ...}}
HeadingInference->>HeadingInference: Merge via defu (override → existing → default)
else No override
HeadingInference->>HeadingInference: Use existing/default settings
end
HeadingInference-->>Content: Inferred/extracted heading
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🌏 Preview Deployments
Built from commit: 🤖 This comment will be updated automatically when you push new commits to this PR. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Updated the `launch.json` to specify distinct names for extension configurations and added a new task in `tasks.json` for `npm: dev:packages+vscode`. Also refined problem matchers for better build tracking.
- Added `basePath` to `NodeUCDStoreOptions` for filesystem root configuration. - Introduced `bridgeBaseUrl` and `storeUrl` in `HTTPUCDStoreOptions` for improved API flexibility. - Updated README and settings to reflect new configuration options.
- Changed `ucd.store-url` to use the correct URL format. - Refactored icon logic in `mapEntryToTreeNode` to use `isDirectory` for determining folder or file icons.
- Changed the default `BASE_URL` for the HTTP fs-bridge to point to `ucd-store.ucdjs.dev`. - Modified path handling in test cases to ensure correct directory structure during listing operations.
- Introduced `ucd.open-in-remote-explorer`, `ucd.open-on-unicode`, and `ucd.open-explorer-entry` commands. - Updated command titles and descriptions in `README.md`. - Enhanced `package.json` with new command configurations. - Added `frontend-url` configuration option for better URL handling. - Refactored `useUCDExplorer` to include `filePath` in UCD item structure.
- Added `useUCDClient` composable to manage UCD client lifecycle. - Updated `UCDContentProvider` to utilize the UCD client for fetching content. - Registered `useUCDClient` in the extension activation process.
- Replaced `ucd.visualize-file` with `ucd.inspect-file` in the command structure. - Implemented the `useInspectFileCommand` to allow inspection of UCD files. - Created `useInspectorView` for displaying inspection results. - Added HTML generation for displaying UCD statistics. - Introduced new pickers for selecting Unicode versions and files. - Removed the unused `visualize-file` command.
Added functions to generate Markdown hover messages for code points, ranges, properties, names, values, and comments in the UCD decorations. This improves the user experience by providing more context and information directly in the editor.
- Implemented `ucd.generate-override` command to facilitate the creation of parser overrides. - Updated command registration and README documentation. - Introduced `useOverrideGenerator` composable for managing selection and generating overrides. - Added HTML generation for user interface during heading selection. - Created schema types for parser overrides to ensure type safety and validation.
- Removed `applyMockDecorations` and replaced it with `useAutoDecorations` for automatic decoration application based on file type. - Introduced new hover information for Unicode properties, code points, ranges, and metadata. - Enhanced the inspector view with improved HTML structure and styling for better readability. - Added support for selection mode in the override generator, allowing users to set start and end lines interactively. - Created a new syntax highlighting file for UCD to improve code readability. This update enhances the user experience by providing contextual information and visual cues directly in the editor.
- Introduced `ucd-selection` view for enhanced selection capabilities. - Updated command execution to focus on the new selection view. - Removed unused selection HTML generation code and refactored related components.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Fix all issues with AI agents
In `@packages/ucd-store/src/factory.ts`:
- Around line 43-49: Update the JSDoc for the HTTP UCD store factory to
reference the actual option name `bridgeBaseUrl` instead of `baseUrl`: adjust
the descriptive text in the block above the function (the comment that documents
`HTTPUCDStoreOptions` / the factory) so the `@param` and inline description
mention `bridgeBaseUrl` and its purpose (override the default bridge base URL).
Keep the rest of the JSDoc intact and only change the option name and its brief
description.
In `@vscode/src/commands/generate-override.ts`:
- Line 79: Replace the brittle split("/") logic that computes fileName from
document.fileName with a platform-agnostic basename call (use Node's
path.basename on document.fileName) and keep the existing fallback to
"Unknown.txt"; add the corresponding import/require for the path module at the
top of generate-override.ts and update the variable assignment that defines
fileName so it uses path.basename(document.fileName) ?? "Unknown.txt".
In `@vscode/src/commands/inspect-file.ts`:
- Around line 18-19: The filename extraction uses document.fileName.split("/")
which fails on Windows; change it to use Node's path utilities or a
cross-platform approach by importing path and calling
path.basename(document.fileName) (or use a regex that handles both "/" and "\"),
updating the code that sets fileName (currently using
document.fileName.split("/") .pop()) to use path.basename(document.fileName) so
filenames are extracted correctly on all platforms.
In `@vscode/src/commands/open-on-unicode.ts`:
- Around line 17-19: The error message inside the scheme check is wrong: when
checking treeViewOrUri.scheme !== "ucd" update the logger.error call to
reference the correct command name (openOnUnicode) and provide context (e.g.,
include the actual scheme or the URI) so the message reads something like
"Invalid URI scheme provided to openOnUnicode command" (optionally appending the
received scheme/treeViewOrUri). Locate the scheme check around treeViewOrUri and
replace the incorrect "openInRemoteExplorer" text in logger.error accordingly.
In `@vscode/src/composables/useInspectorView.ts`:
- Around line 118-122: The rendered header interpolates user-controlled values
(fileName, stats.version, stats.date) directly into the HTML, so add an
escapeHtml helper (escaping &, <, >, and ") in useInspectorView.ts and apply it
to fileName and the stats fields before inserting into the template string
(i.e., replace `${fileName}` with `escapeHtml(fileName)` and likewise for
`stats.version` and `stats.date` in the header template) to prevent HTML
injection and layout breakage.
In `@vscode/src/composables/useOverrideGenerator.ts`:
- Around line 10-35: The three module-scope TextEditorDecorationType instances
(headingHighlightDecoration, selectionStartDecoration, selectionEndDecoration)
are never disposed; export them (or add and export a disposeDecorations()
function) from useOverrideGenerator.ts and ensure the extension's deactivate
handler calls their .dispose() (or calls the exported dispose function) so the
decoration types are properly released instead of only clearing decorations via
cleanup().
In `@vscode/src/composables/useSelectionView.ts`:
- Around line 143-174: User-provided values (fileName, version, jsonPreview) are
interpolated into the HTML and must be escaped to prevent HTML injection; add an
escape helper (e.g., escapeHtml) that replaces &, <, >, and " with their HTML
entities and call it wherever values are injected into the template (replace
${fileName}, ${version}, ${jsonPreview} with ${escapeHtml(fileName)},
${escapeHtml(version)}, ${escapeHtml(jsonPreview)}); keep numeric fields (start,
end) as-is or coerce to numbers before interpolation.
In `@vscode/src/lib/decorations/hovers.ts`:
- Around line 145-158: getDateHover uses new Date(date) which treats date-only
strings in the caller's local timezone and can shift the displayed day; ensure
you parse the incoming date string as a UTC date (or explicitly as midnight UTC)
before formatting so the displayed day is consistent across timezones. Update
getDateHover to normalize the input (for example, append a "T00:00:00Z" or use
Date.UTC/Date constructor with year/month/day parsed from the string) and then
call toLocaleDateString on that UTC-normalized Date object so the formatted
value in the returned MarkdownString is timezone-independent.
In `@vscode/src/lib/ucd-content-provider.ts`:
- Around line 12-14: The non-null assertion on client.value in
provideTextDocumentContent is unsafe; change provideTextDocumentContent to guard
that useUCDClient() returned a client and client.value is defined before calling
client.value.files.get(uri.path). If client.value is undefined, return a
user-friendly error string (or a resolved Promise with an error message) or
throw a handled error instead of using the bang operator; reference useUCDClient
and the files.get call to locate where to add the check and early return/handled
error.
🧹 Nitpick comments (19)
vscode/src/composables/useUCDStore.ts (1)
43-45: Consider using the logger for error output.The error is logged via
console.errorwhile the rest of the file uses the customlogger. For consistency, consider usinglogger.error.Suggested fix
} catch (error) { - console.error("Failed to create UCD store:", error); + logger.error("Failed to create UCD store:", error); store.value = null; }package.json (1)
18-22: LGTM! Minor style inconsistency with filter quoting.The scripts correctly integrate the VS Code extension into the monorepo build system. Note that other filter arguments use quotes (e.g.,
"./packages/*","./apps/*") while these don't. This is functionally fine but slightly inconsistent.Optional: Align quoting style
- "build:vscode": "turbo run build --filter ./vscode", + "build:vscode": "turbo run build --filter \"./vscode\"", ... - "dev:vscode": "turbo run dev --filter ./vscode", + "dev:vscode": "turbo run dev --filter \"./vscode\"",vscode/src/lib/ucd-parser.ts (1)
17-38: Acknowledged WIP: Mock data ingetUCDStats.The function correctly documents its stub status via the TODO comment and underscore-prefixed parameter. Since this is a WIP feature (Inspect File), the mock implementation is acceptable for now.
Consider ensuring the consuming UI (inspector view) clearly indicates when displaying mock/placeholder data, so developers don't mistake these hard-coded values for actual parsed results during testing.
Would you like help implementing the real parsing logic, or should I open an issue to track this TODO?
.vscode/tasks.json (1)
28-51: New combined dev task is well-configured.The task correctly uses
turbo watchwith appropriate filters for packages and vscode. Setting this as the default build task (isDefault: true) makes sense for the combined development workflow.Minor nit: The label
"npm: dev:packages+vscode"suggests an npm script, but the task type is"shell"runningturbodirectly. Consider renaming to"turbo: watch packages+vscode"for clarity.vscode/src/composables/useUCDClient.ts (1)
11-29: Guard condition may prevent client initialization when config is initially undefined.When
immediate: truetriggers the watcher on mount, ifconfig["api-base-url"]starts asundefined, bothnewValandoldValwill beundefined, causing the early return and leavingclient.valueasnull.Consider initializing regardless when
oldValisundefined(first run):♻️ Suggested fix
watch( () => config["api-base-url"], async (newVal, oldVal) => { - if (newVal === oldVal) { + if (oldVal !== undefined && newVal === oldVal) { return; } const createdClient = await tryOr({ try: () => createUCDClient(newVal),vscode/src/composables/useInspectorView.ts (1)
41-51: Hardcoded colors may not match all VS Code themes.The
.file-name,.version, and.dateclasses use hardcoded hex colors (e.g.,#ce9178,#569cd6). These work well with dark themes but may have poor contrast on light themes. Consider using VS Code CSS variables where suitable semantic alternatives exist.vscode/src/lib/pickers.ts (1)
60-71: Promise resolution and disposal pattern works but could be clearer.When the user accepts a selection,
onDidAcceptcallshide(), which triggersonDidHide. The secondresolve(undefined)is harmless (Promises only resolve once), butdispose()correctly runs in both cases.Consider adding a flag or restructuring to make the intent explicit:
♻️ Optional: explicit resolution tracking
return new Promise<string | undefined>((resolve) => { + let resolved = false; quickPick.onDidAccept(() => { const selected = quickPick.selectedItems[0]; + resolved = true; quickPick.hide(); resolve(selected?.filePath); }); quickPick.onDidHide(() => { quickPick.dispose(); - resolve(undefined); + if (!resolved) { + resolve(undefined); + } }); });vscode/src/commands/open-on-unicode.ts (1)
37-46: Remove redundant null checks.After line 32's check confirms
treeViewOrUri.treeItemand__ucdexist, the subsequent checks on lines 38-41 and optional chaining on line 43 are redundant.♻️ Proposed simplification
const ucdItem = (treeViewOrUri.treeItem as UCDTreeItem).__ucd; - if (!ucdItem) { - logger.error("UCD item is undefined or null."); - return; - } - - if (!ucdItem?.url) { + if (!ucdItem.url) { logger.error("UCD item does not have a valid URL."); return; }vscode/src/composables/useSelectionView.ts (1)
246-252: Clarify the purpose of the watchEffect.The
voidexpressions read reactive values solely for their side effect of establishing dependencies. Consider adding a comment to explain this pattern, as it's non-obvious to future maintainers.📝 Suggested comment
watchEffect(() => { + // Read reactive values to establish dependencies and trigger re-renders if (generator.mode.value === "selecting") { void generator.selectionStart.value; void generator.selectionEnd.value; void generator.clickMode.value; } });vscode/syntaxes/ucd.tmLanguage.json (1)
38-41: Field pattern captures surrounding whitespace.The pattern
[^;#\n]+will include leading and trailing spaces around field values. If trimmed highlighting is preferred, consider adjusting the pattern.Optional: Trim whitespace from field matches
"field": { "name": "string.unquoted.field.ucd", - "match": "[^;#\\n]+" + "match": "\\s*([^;#\\n]+?)\\s*(?=[;#\\n]|$)" }Note: This is a nitpick; the current behavior may be acceptable depending on desired visual appearance.
vscode/src/commands/open-in-remote-explorer.ts (2)
33-42: Redundant null check forucdItem.The check at line 33 already validates that
__ucdis truthy, making the subsequent assignment and null check at lines 38-42 redundant.Simplify by removing redundant check
if (!treeViewOrUri.treeItem || !(treeViewOrUri.treeItem as UCDTreeItem).__ucd) { logger.error("Invalid entry provided to openInRemoteExplorer command."); return; } const ucdItem = (treeViewOrUri.treeItem as UCDTreeItem).__ucd; - if (!ucdItem) { - logger.error("UCD item is undefined or null."); - return; - } executeCommand("vscode.open", Uri.parse(`${config["frontend-url"]}/file-explorer/${ucdItem.version}/${hasUCDFolderPath(ucdItem.version) ? "ucd/" : ""}${ucdItem.filePath ?? ""}`));
24-25: Consider awaitingexecuteCommandfor error handling.The
executeCommandcalls are not awaited, which means any errors during command execution won't be caught. If the command can fail (e.g., invalid URL), consider awaiting and handling errors.Optional: Await executeCommand
- executeCommand("vscode.open", Uri.parse(`${config["frontend-url"]}/file-explorer/${treeViewOrUri.path}`)); + await executeCommand("vscode.open", Uri.parse(`${config["frontend-url"]}/file-explorer/${treeViewOrUri.path}`)); return;vscode/src/commands/open-explorer-entry.ts (1)
49-58: Redundant null check forucdItem.Same pattern as
open-in-remote-explorer.ts- the check at line 49 already validates__ucdis truthy.Simplify by removing redundant check
if (!treeView.treeItem || !(treeView.treeItem as UCDTreeItem).__ucd) { logger.error("Invalid entry provided to openEntry command."); return; } const ucdItem = (treeView.treeItem as UCDTreeItem).__ucd; - if (!ucdItem) { - logger.error("UCD item is undefined or null."); - return; - } - if (!ucdItem?.url) { + if (!ucdItem.url) { logger.error("UCD item does not have a valid URL."); return; }vscode/src/lib/decorations/index.ts (1)
50-61: Consider using thedecorationTypesarray for consistency.
applyDecorationsexplicitly lists all 8 decoration types, whileclearDecorationsanddisposeDecorationsiterate overdecorationTypes. If a new type is added, it must be updated in multiple places. Consider restructuring to use a single source of truth.Optional: Use mapped structure for consistency
// In types.ts, export a mapping instead: export const decorationTypeMap = { codePoints: codePointDecorationType, ranges: rangeDecorationType, properties: propertyDecorationType, names: nameDecorationType, values: valueDecorationType, fileNames: fileNameDecorationType, fileVersions: fileVersionDecorationType, fileDates: fileDateDecorationType, } as const; // In index.ts: export function applyDecorations(editor: TextEditor): void { const decorations = parseDocument(editor.document); for (const [key, decorationType] of Object.entries(decorationTypeMap)) { editor.setDecorations(decorationType, decorations[key as keyof ParsedDecorations]); } }This ensures adding a new decoration type only requires updating one location.
vscode/src/lib/decorations/parsers.ts (2)
30-47: Potential index calculation issue inparseMetadataLine.Using
line.indexOf(fullMatch)followed byfullMatch.indexOf(fileMatch[1])could be fragile if the same substring appears multiple times in the line. The regex match already provides index information viafileMatch.index.Suggested improvement using match index
export function parseMetadataLine(line: string): MetadataMatch { const result: MetadataMatch = {}; // Regex: SomeFile-16.0.0.txt const fileMatch = line.match(/#\s*([A-Z]\w*)-(\d+\.\d+\.\d+)(\.txt)/i); - if (fileMatch?.[1] && fileMatch[2]) { - const fullMatch = fileMatch[0]; - const matchStart = line.indexOf(fullMatch); - const afterHash = matchStart + fullMatch.indexOf(fileMatch[1]); - - const fnStart = afterHash; + if (fileMatch?.[1] && fileMatch[2] && fileMatch.index != null) { + const matchStart = fileMatch.index; + // Find position of filename within the match (after "# " prefix) + const fnStart = matchStart + fileMatch[0].indexOf(fileMatch[1]); const fnEnd = fnStart + fileMatch[1].length; result.fileName = { text: fileMatch[1], start: fnStart, end: fnEnd }; const verStart = fnEnd + 1; const verEnd = verStart + fileMatch[2].length; result.version = { text: fileMatch[2], start: verStart, end: verEnd }; }
143-182: Magic number for metadata line limit.The hardcoded
lineIndex <= 5limits metadata parsing to the first 6 lines. This should either be documented or extracted as a constant for clarity.Suggested improvement
+const METADATA_HEADER_LINE_LIMIT = 5; + export function parseDocument(document: TextDocument): ParsedDecorations { // ... for (let lineIndex = 0; lineIndex < lines.length; lineIndex++) { // ... if (line.startsWith("#")) { - if (lineIndex <= 5) { + // Only parse metadata from header comments (first few lines) + if (lineIndex <= METADATA_HEADER_LINE_LIMIT) { const metadata = parseMetadataLine(line);vscode/src/lib/decorations/types.ts (2)
4-4: Mutable exported array is a code smell.Exporting a mutable array that gets populated at module load time can lead to issues if the module is imported before VS Code APIs are ready, or if consumers accidentally mutate it.
Consider using a getter function instead
-export const decorationTypes: TextEditorDecorationType[] = []; +const _decorationTypes: TextEditorDecorationType[] = []; -decorationTypes.push( +_decorationTypes.push( codePointDecorationType, // ... ); +export function getDecorationTypes(): readonly TextEditorDecorationType[] { + return _decorationTypes; +}
6-51: Hardcoded colors may not respect user theme preferences.The decoration types use specific hex colors that work well with dark themes but may have poor contrast on light themes. Consider using theme-aware colors via
ThemeColoror providing configuration options.This is a minor concern for a WIP feature but worth noting for future polish.
vscode/package.json (1)
260-298: Token color customizations override user settings.Using
configurationDefaultsforeditor.tokenColorCustomizationswill apply these rules globally and may override user preferences. This is typically acceptable for extension-specific scopes like*.ucd, but worth documenting.Consider documenting this behavior in the extension README so users know these defaults exist and can override them if needed.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (42)
.github/prompts/unicode-utils-parser-overrides.plan.md.github/workflows/ci.yml.vscode/launch.json.vscode/tasks.jsonpackage.jsonpackages/fs-bridge/playgrounds/http-playground.tspackages/fs-bridge/src/bridges/http.tspackages/fs-bridge/test/bridges/http/http.test.tspackages/ucd-store/src/factory.tsvscode/README.mdvscode/package.jsonvscode/playground/.vscode/settings.jsonvscode/playground/README.mdvscode/src/commands/generate-override.tsvscode/src/commands/index.tsvscode/src/commands/inspect-file.tsvscode/src/commands/open-entry.tsvscode/src/commands/open-explorer-entry.tsvscode/src/commands/open-in-remote-explorer.tsvscode/src/commands/open-on-unicode.tsvscode/src/commands/visualize-file.tsvscode/src/composables/useAutoDecorations.tsvscode/src/composables/useInspectorView.tsvscode/src/composables/useOverrideGenerator.tsvscode/src/composables/useSelectionView.tsvscode/src/composables/useUCDClient.tsvscode/src/composables/useUCDExplorer.tsvscode/src/composables/useUCDStore.tsvscode/src/extension.tsvscode/src/lib/decorations/hovers.tsvscode/src/lib/decorations/index.tsvscode/src/lib/decorations/parsers.tsvscode/src/lib/decorations/types.tsvscode/src/lib/files.tsvscode/src/lib/override-schema.tsvscode/src/lib/pickers.tsvscode/src/lib/ucd-content-provider.tsvscode/src/lib/ucd-parser.tsvscode/src/views/ucd-inspector.tsvscode/src/views/ucd-selection.tsvscode/syntaxes/ucd.tmLanguage.jsonvscode/turbo.json
💤 Files with no reviewable changes (2)
- vscode/src/commands/visualize-file.ts
- vscode/src/commands/open-entry.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{test,spec}.?(c|m)[jt]s?(x)
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.?(c|m)[jt]s?(x): Use test file pattern: **/.{test,spec}.?(c|m)[jt]s?(x)
Import test utilities from path aliases (#test-utils/,#internal/test-utils/conditions) rather than@ucdjs/test-utilsto avoid build dependencies and cyclic dependency issues
Use mockStoreApi() utility from#test-utils/mock-storewith MSW-based mocking for UCD API
Use testdir() from vitest-testdirs to create temporary test directories for filesystem testing
Files:
packages/fs-bridge/test/bridges/http/http.test.ts
🧠 Learnings (24)
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Use ucdjs/schema-gen package to use OpenAI to generate TypeScript type definitions from Unicode data files
Applied to files:
vscode/src/lib/ucd-parser.tsvscode/src/commands/open-on-unicode.tspackages/fs-bridge/playgrounds/http-playground.tsvscode/syntaxes/ucd.tmLanguage.jsonvscode/package.json
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Use Turbo for task orchestration and caching with build dependencies topologically sorted in turbo.json
Applied to files:
vscode/turbo.jsonpackage.json
📚 Learning: 2025-06-09T05:10:32.105Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 45
File: packages/ucd-store/src/download.ts:24-24
Timestamp: 2025-06-09T05:10:32.105Z
Learning: In the ucd-store package refactor, picomatch was moved from direct usage in download.ts to internal usage within the createPathFilter function in filter.ts. The pattern format is still picomatch-compatible, so JSDoc comments referencing picomatch pattern format remain correct.
Applied to files:
packages/fs-bridge/playgrounds/http-playground.tsvscode/src/composables/useUCDStore.tsvscode/src/composables/useUCDExplorer.tsvscode/src/lib/ucd-content-provider.tspackages/ucd-store/src/factory.ts
📚 Learning: 2025-05-04T11:52:22.858Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 6
File: packages/cli/src/cmd/codegen/fields.ts:68-82
Timestamp: 2025-05-04T11:52:22.858Z
Learning: User prefers to address the issue with Node.js API compatibility (`Dirent.parentPath` not being part of Node's API and the `recursive: true` option for `readdir` being only available in Node 21+) in a separate PR rather than the current one.
Applied to files:
packages/fs-bridge/playgrounds/http-playground.tsvscode/src/lib/files.tspackages/fs-bridge/src/bridges/http.tspackages/fs-bridge/test/bridges/http/http.test.ts
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Implement File System Bridge pattern using ucdjs/fs-bridge abstraction layer to support different storage backends (Node.js, HTTP, in-memory)
Applied to files:
packages/fs-bridge/playgrounds/http-playground.tsvscode/src/composables/useUCDStore.tspackages/fs-bridge/test/bridges/http/http.test.tspackages/ucd-store/src/factory.ts
📚 Learning: 2025-06-14T05:20:20.149Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 56
File: packages/utils/src/ucd-files/validate.ts:115-130
Timestamp: 2025-06-14T05:20:20.149Z
Learning: Node.js 22 supports the `recursive` option in `fs.readdir()` (introduced in Node.js 18.17.0), which returns a flattened string array of all descendant files and directories when `recursive: true` is passed.
Applied to files:
packages/fs-bridge/playgrounds/http-playground.tspackages/fs-bridge/src/bridges/http.tspackages/fs-bridge/test/bridges/http/http.test.ts
📚 Learning: 2025-07-13T09:23:43.820Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 107
File: apps/api/src/routes/v1_files.ts:18-32
Timestamp: 2025-07-13T09:23:43.820Z
Learning: UNICODE_STABLE_VERSION from luxass/unicode-utils-new refers to a specific Unicode version string (not a dynamic value), and the validation logic in the files API correctly checks the normalized version (after "latest" substitution) against UNICODE_VERSION_METADATA rather than the mappedVersion from resolveUCDVersion().
Applied to files:
packages/fs-bridge/playgrounds/http-playground.ts
📚 Learning: 2025-06-28T08:01:22.596Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 85
File: packages/fetch/package.json:37-38
Timestamp: 2025-06-28T08:01:22.596Z
Learning: In the ucdjs/ucd project, relative paths in npm scripts within packages (like `../../apps/api/.generated/openapi.json` in packages/fetch/package.json) resolve correctly even when run via Turborepo from the repo root, contrary to initial concerns about working directory changes.
Applied to files:
package.json
📚 Learning: 2025-10-14T07:15:35.199Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 326
File: packages/shared/src/ucd-config.ts:34-58
Timestamp: 2025-10-14T07:15:35.199Z
Learning: In packages/shared/src/ucd-config.ts, the `getDefaultUCDEndpointConfig` function uses a build-time define `__UCD_ENDPOINT_DEFAULT_CONFIG__` that is replaced at build time by the bundler. The `??` operator is evaluated at build time, so the final code contains no runtime branching. Therefore, runtime unit tests for this function are not valuable—the build process itself handles the injection and fallback logic.
Applied to files:
vscode/src/composables/useUCDStore.ts
📚 Learning: 2025-06-29T11:20:13.668Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 87
File: packages/worker-shared/tsconfig.build.json:1-4
Timestamp: 2025-06-29T11:20:13.668Z
Learning: In the ucdjs/ucd project, the packages use tsdown instead of tsc for building libraries. The tsconfig.build.json files are primarily for IDE experience and type checking, not for direct compilation, so including "test" directories in these configs doesn't affect build output.
Applied to files:
.vscode/tasks.jsonvscode/package.json
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Applies to packages/*/tsconfig.json : Use ucdjs-tooling/tsconfig for TypeScript configuration in new packages
Applied to files:
.vscode/tasks.jsonvscode/package.json
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Require Node.js >= 22.18 and pnpm 10.26.1 (enforced via packageManager field in root package.json)
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-07-09T15:05:57.763Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 97
File: packages/utils/src/fs-bridge/node.ts:40-50
Timestamp: 2025-07-09T15:05:57.763Z
Learning: The `parentPath` property on `fs.Dirent` was added in Node.js v20.12, contrary to previous assumptions about it not being part of the Node.js API.
Applied to files:
packages/fs-bridge/src/bridges/http.ts
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Applies to **/*.{test,spec}.?(c|m)[jt]s?(x) : Import test utilities from path aliases (`#test-utils/`*, `#internal/test-utils/conditions`) rather than ucdjs/test-utils to avoid build dependencies and cyclic dependency issues
Applied to files:
packages/fs-bridge/test/bridges/http/http.test.ts
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Applies to **/*.{test,spec}.?(c|m)[jt]s?(x) : Use mockStoreApi() utility from `#test-utils/mock-store` with MSW-based mocking for UCD API
Applied to files:
packages/fs-bridge/test/bridges/http/http.test.ts
📚 Learning: 2025-06-14T05:20:24.527Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 56
File: packages/utils/src/ucd-files/fs-adapter.ts:47-51
Timestamp: 2025-06-14T05:20:24.527Z
Learning: The `recursive` option for `fs.readdir()` was introduced in Node.js v20.1.0 and is available in Node.js 22. When `recursive: true` is passed, it will recursively traverse directories and return all file paths as a flat string array.
Applied to files:
packages/fs-bridge/test/bridges/http/http.test.ts
📚 Learning: 2025-06-14T05:20:24.527Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 56
File: packages/utils/src/ucd-files/fs-adapter.ts:47-51
Timestamp: 2025-06-14T05:20:24.527Z
Learning: In Node.js 22, the `recursive` option for `fs.readdir()` is supported and will recursively find all files and return them in a string array when set to true.
Applied to files:
packages/fs-bridge/test/bridges/http/http.test.ts
📚 Learning: 2025-07-29T06:02:32.422Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 175
File: packages/fs-bridge/test/bridges/node.test.ts:0-0
Timestamp: 2025-07-29T06:02:32.422Z
Learning: In the NodeFileSystemBridge security validation, path traversal detection occurs before critical system paths checks, which means absolute paths to critical system directories will always throw "Path traversal detected" rather than "Critical system path access denied", making it impossible to meaningfully test the critical system paths validation logic.
Applied to files:
packages/fs-bridge/test/bridges/http/http.test.ts
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Note that ucdjs-internal/shared package may not follow semantic versioning as it's only for internal use, and changes may require coordinated updates in other packages
Applied to files:
packages/ucd-store/src/factory.tsvscode/package.json
📚 Learning: 2025-07-20T05:37:40.565Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 131
File: tooling/eslint-plugin/package.json:0-0
Timestamp: 2025-07-20T05:37:40.565Z
Learning: In the ucdjs/ucd project, internal tooling packages (private packages in the tooling/ directory) export TypeScript files directly without requiring a build step, unlike published packages which use tsdown to build and export from ./dist/. Examples include ucdjs/tsdown-config and ucdjs/eslint-plugin.
Applied to files:
vscode/package.json
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Applies to packages/*/tsdown.config.ts : Use ucdjs-tooling/tsdown-config for build configuration in new packages
Applied to files:
vscode/package.json
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Applies to packages/*/tsdown.config.ts : Use tsdown for building TypeScript packages with each package having its own tsdown.config.ts using ucdjs-tooling/tsdown-config
Applied to files:
vscode/package.json
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Applies to packages/*/package.json : Use workspace:* protocol for workspace package dependencies
Applied to files:
vscode/package.json
📚 Learning: 2026-01-08T03:33:34.195Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T03:33:34.195Z
Learning: Use Moonbeam (ucdjs/moonbeam) ESM loader to resolve workspace packages to their source files instead of built versions when running build scripts
Applied to files:
vscode/package.json
🔇 Additional comments (54)
vscode/src/composables/useUCDStore.ts (2)
16-22: LGTM! HTTP store configuration properly wired.The HTTP store creation correctly passes
bridgeBaseUrl(for the FS bridge) andbaseUrl(for API calls) from the extension's configuration. This aligns with the newHTTPUCDStoreOptionsinterface in the factory.
24-31: LGTM! Non-HTTP store now includes baseUrl.The local filesystem store creation now consistently includes
baseUrlfor API calls while using the VS Code FS bridge. This ensures both store variants have access to the API base URL when needed.packages/ucd-store/src/factory.ts (3)
7-10: LGTM! Clean interface design.The
NodeUCDStoreOptionsinterface correctly omitsfsandfsOptions(which are internally managed) while exposingbasePathas an optional configuration. This provides a cleaner API surface for consumers.
28-35: LGTM! Clean options destructuring and spread.The implementation correctly extracts
basePath, resolves it (with a sensible"./"default), and spreads the remaining options to the underlyingcreateUCDStore. This maintains backward compatibility while adding flexibility.
59-68: LGTM! Consistent pattern for HTTP store.The HTTP variant follows the same pattern: destructure bridge-specific option, resolve with fallback to the environment constant, and spread remaining options. Good use of
??for nullish coalescing.packages/fs-bridge/src/bridges/http.ts (1)
136-140: LGTM! Correct fix for recursive path resolution.Using
entry.pathdirectly is the right approach since the API response already contains the full path (e.g.,/dir/subdir/). The previousjoinURL(path, entry.path)would have incorrectly doubled path segments whenentry.pathwas absolute.packages/fs-bridge/playgrounds/http-playground.ts (3)
179-194: Path updates are consistent with new store structure.The changes from
16.0.0/ucd/...to16.0.0/...are applied consistently for both the listdir tests and the path validation logic.
244-256: Complex workflow updated consistently.The discovery and read workflow for UnicodeData.txt correctly reflects the new path structure at
16.0.0/UnicodeData.txt.
22-22: The BASE_URL change tohttps://ucd-store.ucdjs.devis correct.This URL is the documented default for the store subdomain as defined in
packages/env/src/env-constants.ts(JSDM_STORE_BASE_URL). The playground correctly allows this to be overridden via theFS_BRIDGE_HTTP_BASE_URLenvironment variable. The paths used in the file (e.g.,16.0.0/UnicodeData.txt) align with the store's endpoint structure.packages/fs-bridge/test/bridges/http/http.test.ts (3)
1-8: Test imports follow coding guidelines correctly.The test utilities are imported from path aliases (
#internal:bridge/http,#test-utils/msw) as required by the coding guidelines, avoiding build dependencies and cyclic dependency issues. Based on coding guidelines.
176-233: Well-structured test for recursive directory listing.The refactored test correctly validates the recursive listing behavior:
- Mock handlers dynamically construct paths from the request URL using
joinURL(url.pathname, ...), accurately simulating real API behavior.- Expectations correctly reflect full paths (
/dir/file1.txt,/dir/subdir/nested.txt), validating that the bridge properly usesentry.pathfrom API responses.This test structure provides better coverage for the path resolution fix in
http.ts.
276-317: Inaccessible subdirectory test paths updated correctly.The path updates in the mock responses and assertions ensure consistency with the new path resolution behavior. The test correctly verifies that forbidden subdirectories result in empty children arrays without propagating the error.
vscode/playground/README.md (1)
1-11: LGTM!Clear and helpful documentation for the extension development workflow. This will aid developer onboarding and ensure consistent debugging practices.
vscode/turbo.json (1)
5-14: LGTM!Adding
$TURBO_EXTENDS$to the build task'sdependsOnensures proper inheritance from the root turbo.json configuration, which is consistent with how thetypechecktask is already configured. This properly integrates the VS Code extension build into the monorepo's task graph. Based on learnings, this aligns with the practice of using Turbo for task orchestration with build dependencies topologically sorted.vscode/src/composables/useUCDExplorer.ts (2)
10-16: LGTM!Adding the optional
filePathproperty extends the interface to support the new file navigation features while maintaining backward compatibility.
81-85: Good refinement of the data shape.Narrowing
__ucdto onlyversionandurlinstead of spreading the full metadata object is a cleaner approach—it explicitly documents the expected shape and avoids exposing unnecessary internal data to consumers.vscode/src/lib/ucd-parser.ts (1)
1-15: LGTM!The
UCDStatsinterface is well-structured with appropriate types for the statistics data.vscode/playground/.vscode/settings.json (1)
1-9: LGTM!The playground settings provide sensible local development defaults. The use of
localhostURLs and empty store filters is appropriate for a development/testing environment.vscode/src/lib/ucd-content-provider.ts (1)
16-26: LGTM on error handling.The error handling approach—returning descriptive error messages as document content—is appropriate for a
TextDocumentContentProvider. The type validation on line 21-24 is good defensive programming..github/workflows/ci.yml (3)
53-73: LGTM! Newbuild-vscodejob aligns with PR objectives.The job structure is consistent with existing
build-packagesandbuild-appsjobs. This addresses the CI coverage objective from issue#423.
97-102: Dependency graph looks correct.The
lintjob correctly depends on all three build jobs. The redundant build commands on line 121 should be fast due to Turbo's remote caching (configured viaTURBO_TOKEN/TURBO_TEAM).Also applies to: 120-121
126-131: Consistent with lint job updates.Same pattern applied to
typecheckjob—dependencies and build commands mirror thelintjob configuration.Also applies to: 149-150
.vscode/tasks.json (1)
13-22: LGTM on problem matcher configuration.The background patterns correctly match the vscode-ucd build output format, and using
$ts-webpack-watchas the base is appropriate for TypeScript watch builds.vscode/README.md (3)
9-17: LGTM!The commands table accurately reflects the new commands introduced in this PR. The documentation is clear and well-formatted.
25-31: LGTM!The configurations table properly documents the new URL-based settings (
api-base-url,store-url,frontend-url) with appropriate defaults.
39-41: LGTM!The new
ucdlanguage entry with its.ucdextension andsource.ucdgrammar scope is properly documented..vscode/launch.json (1)
4-31: LGTM!The two launch configurations provide good flexibility for development workflows — "Extension Only" for quick iteration during extension-focused work, and "Full Watch" for testing with dependent packages rebuilt. The playground folder integration aligns well with the PR's DX improvements.
vscode/src/composables/useInspectorView.ts (1)
257-290: LGTM!The composable follows a clean reactive pattern — computed stats derived from content, conditional HTML rendering, and a clear API surface with
inspectFile,clear,fileName, andstats.vscode/src/lib/files.ts (1)
16-41: LGTM!Good refactor using
entry.type === "directory"for explicit type checking instead of inferring from children. The conditional command attachment and newfilePathproperty integrate well with the open-explorer-entry command flow.vscode/src/lib/pickers.ts (1)
16-30: LGTM!Clean implementation of the version picker with good metadata display (version label, draft indicator, release date).
vscode/src/lib/override-schema.ts (1)
1-51: LGTM!The schema types and helper functions are well-structured. The validation logic in
isValidPositioncorrectly handles edge cases (non-negative integers, end >= start). The conditional spread pattern increateParserOverrideis a clean way to optionally include the heading.vscode/src/composables/useOverrideGenerator.ts (2)
80-104: Selection change handling logic is sound.The boundary normalization when clicking out of order (e.g., setting start after end) correctly swaps the values. The toggle between
set-startandset-endmodes provides clear UX.
162-175: Cleanup properly disposes resources.The cleanup function correctly resets all state, clears decorations, and disposes the selection change listener. Good resource management.
.github/prompts/unicode-utils-parser-overrides.plan.md (1)
1-469: Well-structured implementation plan.This planning document provides a comprehensive blueprint for the parser override system, covering schema design, registry, filesystem loader, integration points, tests, and migration strategy. The design correctly addresses browser compatibility (registry-only) vs Node.js (filesystem loading) and maintains backward compatibility with the deprecated
HEADING_SETTINGS_CONFIG.vscode/src/views/ucd-inspector.ts (1)
1-5: LGTM!Clean wrapper pattern that maintains consistency with other view initializers in the codebase. The delegation to the composable keeps the initialization logic centralized.
vscode/src/views/ucd-selection.ts (1)
1-5: LGTM!Consistent with the inspector view initializer pattern. The symmetry between these view wrappers makes the codebase easier to navigate.
vscode/src/commands/index.ts (1)
1-17: LGTM!Clean command registration refactor. All imported command hooks are properly invoked, and the naming convention is consistent throughout.
vscode/src/extension.ts (1)
11-23: LGTM!The activation sequence logically orders view initialization before command registration and client setup. The initialization flow follows the reactive-vscode pattern appropriately.
vscode/src/commands/inspect-file.ts (1)
6-26: Command implementation looks good overall.The reactive pattern usage is correct, error feedback for missing editor is appropriate, and the flow from inspection to focus is logical.
vscode/src/composables/useAutoDecorations.ts (1)
4-22: LGTM on the reactive pattern.The composable correctly uses reactive-vscode hooks to track editor changes and applies/clears decorations based on document context. The
watchEffectensures decorations stay synchronized with the active editor.vscode/syntaxes/ucd.tmLanguage.json (1)
1-43: Grammar structure looks good overall.The TextMate grammar correctly handles UCD file syntax with appropriate scopes for comments, codepoints, ranges, separators, and fields. The pattern ordering (range before codepoint) ensures longer matches take precedence.
vscode/src/commands/open-in-remote-explorer.ts (1)
10-46: Good input validation and error logging.The command properly validates both Uri and TreeViewNode inputs with appropriate error messages. The use of
hasUCDFolderPathfor version-specific path construction is consistent with other commands in the PR.vscode/src/commands/open-explorer-entry.ts (3)
46-67: TreeViewNode path opens raw URL vs constructingucd:URI.The TreeViewNode branch uses
executeCommand("vscode.open", Uri.parse(ucdItem.url))which may open a remote URL, whileopenUCDFileconstructs aucd:scheme URI. This appears intentional (tree items may have external URLs), but verify this is the desired behavior.
11-18: Acknowledged workaround for language setting limitation.The comment explaining the
setTextDocumentLanguageside effect (close/open events) is helpful. This is a known VS Code API limitation.
20-77: Command structure is well-organized.The three invocation styles (picker flow, TreeViewNode, version+filePath) are cleanly separated with appropriate validation. The use of
toRawto unwrap the reactive store is correct.vscode/src/lib/decorations/index.ts (1)
1-73: Clean module structure with well-defined public API.The file provides a clear orchestration layer with appropriate re-exports. The separation into hovers, parsers, and types submodules is well-organized.
vscode/src/commands/generate-override.ts (2)
23-35: Version extraction looks reasonable with appropriate fallback.The function correctly handles two patterns:
Unicode X.Y.ZandDate: YYYY, with a sensible default. The regex patterns are appropriate for the use case.
41-91: Command flow and user interaction are well-structured.The two-phase interaction (start selection → confirm/cancel) is clear. The use of quick pick for confirmation with descriptive labels provides good UX. The clipboard integration for JSON output is practical.
vscode/src/lib/decorations/parsers.ts (1)
128-236: Overall parsing logic is sound.The document parsing correctly separates metadata lines (comments) from data lines, applies appropriate decoration types based on field indices, and builds a comprehensive decoration structure. The use of
lineAtfor range construction is appropriate.vscode/src/lib/decorations/hovers.ts (2)
4-41: Good coverage of Unicode property descriptions.The
PROPERTY_DESCRIPTIONSmap covers common General Category values and Bidi joining types. This will provide helpful context for users exploring UCD files.
128-143: Helpful version hover with relevant links.The version hover provides useful links to Unicode.org resources and the project's own explorer. The URL construction looks correct for standard Unicode version formats.
vscode/package.json (3)
106-127: Configuration schema usesanyOffor flexible URL input.The pattern of providing predefined enum values alongside a freeform URL option is a good UX choice, allowing users to select known endpoints or specify custom ones.
223-238: Menu enablement useseditorLangId == ucdfor editor/title commands.This is appropriate for the language-specific commands. However, the
ucd.inspect-filecommand also hasenablement": "resourceScheme == ucd"in its definition (line 42), which could conflict or be redundant with the menuwhenclause.Consider whether both conditions are needed or if they should be unified for clarity.
310-316: Dependencies look appropriate for the extension scope.The workspace dependencies follow the project's conventions using
workspace:*protocol. The addition of@ucdjs-internal/sharedaligns with the learnings about internal package usage in this project.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /** | ||
| * Creates a new UCD store instance configured for HTTP access. | ||
| * | ||
| * This function simplifies the creation of an HTTP UCD store by: | ||
| * - Automatically loading the HTTP file system bridge with read-only capabilities | ||
| * - Configuring for remote data access via the store subdomain (ucd-store.ucdjs.dev) | ||
| * - Initializing the store with the specified options | ||
| * | ||
| * The HTTP store uses the dedicated store subdomain which provides direct access | ||
| * to version files without requiring the /ucd/ path prefix. | ||
| * | ||
| * @param {Omit<UCDStoreOptions, "fs">} options - Configuration options for the HTTP UCD store | ||
| * @returns {Promise<UCDStore>} A fully initialized UCDStore instance with HTTP filesystem capabilities | ||
| * Creates a UCD store backed by the HTTP FileSystemBridge. | ||
| * @template BridgeOptionsSchema extends z.ZodType | ||
| * @param {HTTPUCDStoreOptions<BridgeOptionsSchema>} [options] Store options; provide baseUrl to override the default. | ||
| * @returns {Promise<UCDStore>} A ready-to-use store instance. | ||
| * @throws {Error} If the HTTP FileSystemBridge could not be loaded. | ||
| */ |
There was a problem hiding this comment.
JSDoc references incorrect option name.
The JSDoc on line 46 mentions baseUrl but the actual option is bridgeBaseUrl.
Suggested fix
/**
* Creates a UCD store backed by the HTTP FileSystemBridge.
* `@template` BridgeOptionsSchema extends z.ZodType
- * `@param` {HTTPUCDStoreOptions<BridgeOptionsSchema>} [options] Store options; provide baseUrl to override the default.
+ * `@param` {HTTPUCDStoreOptions<BridgeOptionsSchema>} [options] Store options; provide bridgeBaseUrl to override the default.
* `@returns` {Promise<UCDStore>} A ready-to-use store instance.
* `@throws` {Error} If the HTTP FileSystemBridge could not be loaded.
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Creates a new UCD store instance configured for HTTP access. | |
| * | |
| * This function simplifies the creation of an HTTP UCD store by: | |
| * - Automatically loading the HTTP file system bridge with read-only capabilities | |
| * - Configuring for remote data access via the store subdomain (ucd-store.ucdjs.dev) | |
| * - Initializing the store with the specified options | |
| * | |
| * The HTTP store uses the dedicated store subdomain which provides direct access | |
| * to version files without requiring the /ucd/ path prefix. | |
| * | |
| * @param {Omit<UCDStoreOptions, "fs">} options - Configuration options for the HTTP UCD store | |
| * @returns {Promise<UCDStore>} A fully initialized UCDStore instance with HTTP filesystem capabilities | |
| * Creates a UCD store backed by the HTTP FileSystemBridge. | |
| * @template BridgeOptionsSchema extends z.ZodType | |
| * @param {HTTPUCDStoreOptions<BridgeOptionsSchema>} [options] Store options; provide baseUrl to override the default. | |
| * @returns {Promise<UCDStore>} A ready-to-use store instance. | |
| * @throws {Error} If the HTTP FileSystemBridge could not be loaded. | |
| */ | |
| /** | |
| * Creates a UCD store backed by the HTTP FileSystemBridge. | |
| * `@template` BridgeOptionsSchema extends z.ZodType | |
| * `@param` {HTTPUCDStoreOptions<BridgeOptionsSchema>} [options] Store options; provide bridgeBaseUrl to override the default. | |
| * `@returns` {Promise<UCDStore>} A ready-to-use store instance. | |
| * `@throws` {Error} If the HTTP FileSystemBridge could not be loaded. | |
| */ |
🤖 Prompt for AI Agents
In `@packages/ucd-store/src/factory.ts` around lines 43 - 49, Update the JSDoc for
the HTTP UCD store factory to reference the actual option name `bridgeBaseUrl`
instead of `baseUrl`: adjust the descriptive text in the block above the
function (the comment that documents `HTTPUCDStoreOptions` / the factory) so the
`@param` and inline description mention `bridgeBaseUrl` and its purpose
(override the default bridge base URL). Keep the rest of the JSDoc intact and
only change the option name and its brief description.
| function detectHeadingBounds(content: string): { start: number; end: number } { | ||
| const lines = content.split("\n"); | ||
| let end = 0; | ||
|
|
||
| for (let i = 0; i < lines.length; i++) { | ||
| const line = lines[i]?.trim() ?? ""; | ||
|
|
||
| if (line === "" || line.startsWith("#")) { | ||
| end = i; | ||
| } else { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| return { start: 0, end }; | ||
| } |
There was a problem hiding this comment.
Off-by-one in heading bounds detection.
The function returns end as the last line index where a blank/comment was found, but the message on line 89 displays lines ${detected.start}-${detected.end} suggesting it's a range. If end is meant to be inclusive of the heading block, it should be end + 1 or the logic should clarify whether end is inclusive or exclusive.
Also, if the first line is non-empty and not a comment, end remains 0, making start: 0, end: 0 potentially confusing (is this an empty range or a single-line heading?).
Suggested clarification
function detectHeadingBounds(content: string): { start: number; end: number } {
const lines = content.split("\n");
- let end = 0;
+ let end = -1; // -1 indicates no heading found
for (let i = 0; i < lines.length; i++) {
const line = lines[i]?.trim() ?? "";
if (line === "" || line.startsWith("#")) {
end = i;
} else {
break;
}
}
- return { start: 0, end };
+ // Return end as exclusive (end + 1) for clearer semantics
+ return { start: 0, end: end + 1 };
}|
|
||
| const document = editor.document; | ||
| const content = document.getText(); | ||
| const fileName = document.fileName.split("/").pop() ?? "Unknown.txt"; |
There was a problem hiding this comment.
Cross-platform path handling issue.
Using split("/") won't work correctly on Windows where paths use backslashes. Consider using a platform-agnostic approach.
Suggested fix
- const fileName = document.fileName.split("/").pop() ?? "Unknown.txt";
+ const fileName = document.uri.path.split("/").pop() ?? document.fileName.split(/[\\/]/).pop() ?? "Unknown.txt";Or use Node's path.basename if available in the extension context.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fileName = document.fileName.split("/").pop() ?? "Unknown.txt"; | |
| const fileName = document.uri.path.split("/").pop() ?? document.fileName.split(/[\\/]/).pop() ?? "Unknown.txt"; |
🤖 Prompt for AI Agents
In `@vscode/src/commands/generate-override.ts` at line 79, Replace the brittle
split("/") logic that computes fileName from document.fileName with a
platform-agnostic basename call (use Node's path.basename on document.fileName)
and keep the existing fallback to "Unknown.txt"; add the corresponding
import/require for the path module at the top of generate-override.ts and update
the variable assignment that defines fileName so it uses
path.basename(document.fileName) ?? "Unknown.txt".
| const document = editor.document; | ||
| const fileName = document.fileName.split("/").pop() ?? "Unknown File"; |
There was a problem hiding this comment.
Cross-platform path handling issue.
Using split("/") won't correctly extract the filename on Windows, where paths use backslashes. The entire path would be returned instead of just the filename.
🔧 Suggested fix using path module or regex
+import { basename } from "node:path";
+
// In the handler:
- const fileName = document.fileName.split("/").pop() ?? "Unknown File";
+ const fileName = basename(document.fileName) || "Unknown File";Alternatively, if you want to avoid adding an import:
- const fileName = document.fileName.split("/").pop() ?? "Unknown File";
+ const fileName = document.fileName.split(/[\\/]/).pop() ?? "Unknown File";🤖 Prompt for AI Agents
In `@vscode/src/commands/inspect-file.ts` around lines 18 - 19, The filename
extraction uses document.fileName.split("/") which fails on Windows; change it
to use Node's path utilities or a cross-platform approach by importing path and
calling path.basename(document.fileName) (or use a regex that handles both "/"
and "\"), updating the code that sets fileName (currently using
document.fileName.split("/") .pop()) to use path.basename(document.fileName) so
filenames are extracted correctly on all platforms.
| if (treeViewOrUri.scheme !== "ucd") { | ||
| logger.error("Invalid URI scheme provided to openInRemoteExplorer command."); | ||
| return; |
There was a problem hiding this comment.
Fix the incorrect error message.
The error message references openInRemoteExplorer but this is the openOnUnicode command.
✏️ Proposed fix
if (treeViewOrUri.scheme !== "ucd") {
- logger.error("Invalid URI scheme provided to openInRemoteExplorer command.");
+ logger.error("Invalid URI scheme provided to openOnUnicode command.");
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (treeViewOrUri.scheme !== "ucd") { | |
| logger.error("Invalid URI scheme provided to openInRemoteExplorer command."); | |
| return; | |
| if (treeViewOrUri.scheme !== "ucd") { | |
| logger.error("Invalid URI scheme provided to openOnUnicode command."); | |
| return; |
🤖 Prompt for AI Agents
In `@vscode/src/commands/open-on-unicode.ts` around lines 17 - 19, The error
message inside the scheme check is wrong: when checking treeViewOrUri.scheme !==
"ucd" update the logger.error call to reference the correct command name
(openOnUnicode) and provide context (e.g., include the actual scheme or the URI)
so the message reads something like "Invalid URI scheme provided to
openOnUnicode command" (optionally appending the received scheme/treeViewOrUri).
Locate the scheme check around treeViewOrUri and replace the incorrect
"openInRemoteExplorer" text in logger.error accordingly.
| <section class="section"> | ||
| <div class="section-title">Target File</div> | ||
| <div class="info-row"> | ||
| <span class="info-label">File</span> | ||
| <span class="info-value">${fileName}</span> | ||
| </div> | ||
| <div class="info-row"> | ||
| <span class="info-label">Version</span> | ||
| <span class="info-value">${version}</span> | ||
| </div> | ||
| </section> | ||
|
|
||
| <section class="section"> | ||
| <div class="section-title">Current Selection</div> | ||
| <div class="info-row"> | ||
| <span class="info-label">Start Line</span> | ||
| <span class="selection-badge">${start}</span> | ||
| </div> | ||
| <div class="info-row"> | ||
| <span class="info-label">End Line</span> | ||
| <span class="selection-badge">${end}</span> | ||
| </div> | ||
| <div class="info-row"> | ||
| <span class="info-label">Total Lines</span> | ||
| <span class="info-value">${end - start + 1}</span> | ||
| </div> | ||
| </section> | ||
|
|
||
| <section class="section"> | ||
| <div class="section-title">JSON Preview</div> | ||
| <div class="json-preview">${jsonPreview}</div> | ||
| </section> |
There was a problem hiding this comment.
Escape user-provided values to prevent HTML injection.
The fileName, version, and jsonPreview values are interpolated directly into HTML. While scripts are disabled in the webview, malicious HTML tags could still disrupt the UI or create misleading content.
🔒 Proposed fix: Add an escape helper
function escapeHtml(str: string): string {
return str
.replace(/&/g, "&")
.replace(/</g, "<")
.replace(/>/g, ">")
.replace(/"/g, """);
}Then use it for interpolated values:
- <span class="info-value">${fileName}</span>
+ <span class="info-value">${escapeHtml(fileName)}</span>Apply similarly to version and jsonPreview.
🤖 Prompt for AI Agents
In `@vscode/src/composables/useSelectionView.ts` around lines 143 - 174,
User-provided values (fileName, version, jsonPreview) are interpolated into the
HTML and must be escaped to prevent HTML injection; add an escape helper (e.g.,
escapeHtml) that replaces &, <, >, and " with their HTML entities and call it
wherever values are injected into the template (replace ${fileName}, ${version},
${jsonPreview} with ${escapeHtml(fileName)}, ${escapeHtml(version)},
${escapeHtml(jsonPreview)}); keep numeric fields (start, end) as-is or coerce to
numbers before interpolation.
| export function getCodePointHover(codePoint: string): MarkdownString { | ||
| const cp = Number.parseInt(codePoint, 16); | ||
| const char = String.fromCodePoint(cp); | ||
| const displayChar = cp >= 0x20 && cp <= 0x7E ? char : cp >= 0x100 ? char : "(control character)"; | ||
|
|
||
| return createMarkdown([ | ||
| `### Unicode Code Point`, | ||
| ``, | ||
| `| Property | Value |`, | ||
| `|----------|-------|`, | ||
| `| **Hex** | U+${codePoint.toUpperCase().padStart(4, "0")} |`, | ||
| `| **Decimal** | ${cp} |`, | ||
| `| **Character** | ${displayChar} |`, | ||
| ].join("\n")); | ||
| } |
There was a problem hiding this comment.
Edge case in character display logic.
The condition cp >= 0x20 && cp <= 0x7E handles printable ASCII, and cp >= 0x100 allows most non-ASCII characters, but code points in range 0x7F-0xFF (extended ASCII control/special chars) will display as "(control character)" even though many are printable in Latin-1 (e.g., 0xA9 = ©).
Consider a more comprehensive check
- const displayChar = cp >= 0x20 && cp <= 0x7E ? char : cp >= 0x100 ? char : "(control character)";
+ // Printable: ASCII printable (0x20-0x7E), Latin-1 Supplement printable (0xA0-0xFF), or higher
+ const isPrintable = (cp >= 0x20 && cp <= 0x7E) || cp >= 0xA0;
+ const displayChar = isPrintable ? char : "(control character)";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getCodePointHover(codePoint: string): MarkdownString { | |
| const cp = Number.parseInt(codePoint, 16); | |
| const char = String.fromCodePoint(cp); | |
| const displayChar = cp >= 0x20 && cp <= 0x7E ? char : cp >= 0x100 ? char : "(control character)"; | |
| return createMarkdown([ | |
| `### Unicode Code Point`, | |
| ``, | |
| `| Property | Value |`, | |
| `|----------|-------|`, | |
| `| **Hex** | U+${codePoint.toUpperCase().padStart(4, "0")} |`, | |
| `| **Decimal** | ${cp} |`, | |
| `| **Character** | ${displayChar} |`, | |
| ].join("\n")); | |
| } | |
| export function getCodePointHover(codePoint: string): MarkdownString { | |
| const cp = Number.parseInt(codePoint, 16); | |
| const char = String.fromCodePoint(cp); | |
| // Printable: ASCII printable (0x20-0x7E), Latin-1 Supplement printable (0xA0-0xFF), or higher | |
| const isPrintable = (cp >= 0x20 && cp <= 0x7E) || cp >= 0xA0; | |
| const displayChar = isPrintable ? char : "(control character)"; | |
| return createMarkdown([ | |
| `### Unicode Code Point`, | |
| ``, | |
| `| Property | Value |`, | |
| `|----------|-------|`, | |
| `| **Hex** | U+${codePoint.toUpperCase().padStart(4, "0")} |`, | |
| `| **Decimal** | ${cp} |`, | |
| `| **Character** | ${displayChar} |`, | |
| ].join("\n")); | |
| } |
| export function getDateHover(date: string): MarkdownString { | ||
| const formatted = new Date(date).toLocaleDateString("en-US", { | ||
| weekday: "long", | ||
| year: "numeric", | ||
| month: "long", | ||
| day: "numeric", | ||
| }); | ||
|
|
||
| return createMarkdown([ | ||
| `### File Date`, | ||
| ``, | ||
| `**${formatted}**`, | ||
| ].join("\n")); | ||
| } |
There was a problem hiding this comment.
Date parsing without timezone consideration.
new Date(date) parses the date string in local timezone, which may produce unexpected results. For a date string like "2024-01-15", the result depends on the user's timezone.
Consider explicit UTC handling
export function getDateHover(date: string): MarkdownString {
- const formatted = new Date(date).toLocaleDateString("en-US", {
+ // Append time to force UTC interpretation for date-only strings
+ const dateObj = new Date(`${date}T00:00:00Z`);
+ const formatted = dateObj.toLocaleDateString("en-US", {
weekday: "long",
year: "numeric",
month: "long",
day: "numeric",
+ timeZone: "UTC",
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getDateHover(date: string): MarkdownString { | |
| const formatted = new Date(date).toLocaleDateString("en-US", { | |
| weekday: "long", | |
| year: "numeric", | |
| month: "long", | |
| day: "numeric", | |
| }); | |
| return createMarkdown([ | |
| `### File Date`, | |
| ``, | |
| `**${formatted}**`, | |
| ].join("\n")); | |
| } | |
| export function getDateHover(date: string): MarkdownString { | |
| // Append time to force UTC interpretation for date-only strings | |
| const dateObj = new Date(`${date}T00:00:00Z`); | |
| const formatted = dateObj.toLocaleDateString("en-US", { | |
| weekday: "long", | |
| year: "numeric", | |
| month: "long", | |
| day: "numeric", | |
| timeZone: "UTC", | |
| }); | |
| return createMarkdown([ | |
| `### File Date`, | |
| ``, | |
| `**${formatted}**`, | |
| ].join("\n")); | |
| } |
🤖 Prompt for AI Agents
In `@vscode/src/lib/decorations/hovers.ts` around lines 145 - 158, getDateHover
uses new Date(date) which treats date-only strings in the caller's local
timezone and can shift the displayed day; ensure you parse the incoming date
string as a UTC date (or explicitly as midnight UTC) before formatting so the
displayed day is consistent across timezones. Update getDateHover to normalize
the input (for example, append a "T00:00:00Z" or use Date.UTC/Date constructor
with year/month/day parsed from the string) and then call toLocaleDateString on
that UTC-normalized Date object so the formatted value in the returned
MarkdownString is timezone-independent.
| const parts = line.split(";"); | ||
| const firstPart = parts[0]; | ||
| if (parts.length >= 2 && firstPart != null) { | ||
| let currentOffset = firstPart.length + 1; | ||
|
|
||
| for (let i = 1; i < parts.length; i++) { | ||
| const part = parts[i]; | ||
| if (part == null) continue; | ||
|
|
||
| const val = part.trim(); | ||
| if (val.length > 0) { | ||
| const valStart = line.indexOf(part, currentOffset - 1); | ||
| result.fields.push({ | ||
| text: part, | ||
| start: valStart, | ||
| end: valStart + part.length, | ||
| index: i, | ||
| }); | ||
| } | ||
| currentOffset += part.length + 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
Field offset calculation could drift on edge cases.
The currentOffset tracking assumes sequential parsing, but line.indexOf(part, currentOffset - 1) with currentOffset - 1 looks like a workaround. If part contains leading whitespace, indexOf returns the position including whitespace, but val is trimmed, causing a mismatch between what's stored (part with whitespace) and what's checked (val.length > 0).
Consider using the actual untrimmed part for position calculation and storing trimmed text separately.
There was a problem hiding this comment.
Pull request overview
This PR enhances the developer experience of the VSCode extension for working with Unicode Character Database (UCD) files by adding comprehensive features for viewing, inspecting, and generating parser overrides.
Changes:
- Added TextMate grammar for UCD file syntax highlighting
- Implemented inspector and selection views for file analysis and heading selection
- Added commands for opening files on unicode.org and ucdjs.dev, inspecting files, and generating parser overrides
- Introduced automatic decorations for UCD files with hover information
- Refactored UCD client creation and content provider
- Enhanced build configuration and CI/CD workflows
- Added playground workspace for consistent extension development
Reviewed changes
Copilot reviewed 42 out of 43 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| vscode/turbo.json | Added turbo extends dependency for build task |
| vscode/syntaxes/ucd.tmLanguage.json | New TextMate grammar for UCD file syntax highlighting |
| vscode/src/views/* | New inspector and selection view implementations |
| vscode/src/lib/* | UCD parser, override schema, decorations, pickers, and file utilities |
| vscode/src/composables/* | New composables for client, views, decorations, and override generation |
| vscode/src/commands/* | Refactored and new commands for file operations |
| vscode/package.json | Updated commands, views, configuration, and grammars |
| vscode/playground/* | New development workspace for extension testing |
| packages/ucd-store/src/factory.ts | Refactored store factory with improved documentation |
| packages/fs-bridge/src/bridges/http.ts | Fixed path handling in recursive listdir |
| .vscode/* | Enhanced launch and tasks configuration |
| .github/workflows/ci.yml | Added vscode build job to CI |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const isUcdFile = doc.uri.scheme === "ucd" | ||
| || doc.languageId === "ucd" | ||
| || doc.fileName.endsWith(".txt"); |
There was a problem hiding this comment.
This condition will apply decorations to ALL .txt files, not just UCD files. This is overly broad and will incorrectly apply UCD-specific decorations to any text file opened in VSCode. Consider removing this condition or making it more specific (e.g., checking if the file path contains 'unicode' or 'ucd').
| const isUcdFile = doc.uri.scheme === "ucd" | |
| || doc.languageId === "ucd" | |
| || doc.fileName.endsWith(".txt"); | |
| const lowerCaseFileName = doc.fileName.toLowerCase(); | |
| const isUcdFile = doc.uri.scheme === "ucd" | |
| || doc.languageId === "ucd" | |
| || (lowerCaseFileName.endsWith(".txt") && (lowerCaseFileName.includes("ucd") || lowerCaseFileName.includes("unicode"))); |
| return `${dateMatch[1]}.0.0`; | ||
| } | ||
|
|
||
| return "16.0.0"; |
There was a problem hiding this comment.
The hardcoded fallback version '16.0.0' may become outdated as new Unicode versions are released. Consider using a constant defined elsewhere (e.g., from @unicode-utils/core) or making this configurable, so it's easier to update when new versions are released.
| // TODO: This would allow to traverse upwards, this should be blocked. | ||
| executeCommand("vscode.open", Uri.parse(`https://unicode.org/Public/${treeViewOrUri.path}`)); |
There was a problem hiding this comment.
The TODO comment indicates a known path traversal vulnerability. The URI path should be validated to prevent upward directory traversal (e.g., '../../../etc/passwd'). Use path normalization and validation before constructing the URL.
| const [data, err] = await store.files.tree(version); | ||
|
|
||
| if (err) { | ||
| throw err; | ||
| } |
There was a problem hiding this comment.
The error is caught in the outer try-catch and logged, but the original error type information may be lost when thrown. Consider logging the error with more context before throwing, or handle specific error types differently if needed.
| logger.error("UCD item does not have a valid URL."); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
This code path opens ucdItem.url when called with a TreeViewNode, but the command is named 'openExplorerEntry' which suggests it should open the file in the UCD viewer (using the 'ucd:' scheme) rather than opening an external URL. This behavior seems inconsistent with the command at line 75 which calls openUCDFile(). The TreeViewNode path should likely also open in the UCD viewer.
| // Prefer opening UCD files in the internal UCD viewer when possible. | |
| try { | |
| const parsedUrl = new URL(ucdItem.url); | |
| if ((parsedUrl.protocol === "http:" || parsedUrl.protocol === "https:") && parsedUrl.hostname === "www.unicode.org") { | |
| const segments = parsedUrl.pathname.split("/").filter(Boolean); | |
| // Expected pattern: /Public/<version>/[...file path...] | |
| if (segments[0] === "Public" && segments.length >= 3) { | |
| const version = segments[1]; | |
| const filePath = segments.slice(2).join("/"); | |
| await openUCDFile(version, filePath); | |
| return; | |
| } | |
| } | |
| } catch (error) { | |
| logger.warn?.("Failed to parse UCD item URL; falling back to default opener.", error as Error); | |
| } |
| export interface NodeUCDStoreOptions<BridgeOptionsSchema extends z.ZodType> | ||
| extends Omit<UCDStoreOptions<BridgeOptionsSchema>, "fs" | "fsOptions"> { | ||
| basePath?: string; |
There was a problem hiding this comment.
The new interface improves type safety and code organization. However, the JSDoc comment for createNodeUCDStore should be updated to reference this new interface type in the @param tag instead of describing it inline.
| export function getUCDStats(_content: string): UCDStats { | ||
| // TODO: Implement real parsing |
There was a problem hiding this comment.
The function is documented as work-in-progress with a TODO comment and underscore-prefixed parameter indicating it's unused. While this is clearly marked, consider adding a JSDoc tag like @todo Implement real parsing and possibly a console warning when this mock data is returned so developers are aware it's not yet functional.
🔗 Linked issue
resolves #423
📚 Description
This PR is a combination of many things. The overall goal is to make the dx of the extension better.
Therefor, the following things has been semi implemented:
Summary by CodeRabbit
New Features
Infrastructure
✏️ Tip: You can customize this high-level summary in your review settings.