-
-
Notifications
You must be signed in to change notification settings - Fork 16
Release 1.8.0 #223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release 1.8.0 #223
Conversation
move demo time to nx
regenerate package-lock.json
…ypingService #release
|
Warning Rate limit exceeded@estruyf has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis update introduces a new "hacker-typer" typing mode for code insertion, replacement, and patch actions, enabling text to be inserted in user-triggered chunks. It adds a new API endpoint to list available demos, updates configuration and command schemas, enhances documentation, tests, and status bar integration, and improves terminal and chat action handling. Several files are reformatted or updated for consistency, and a new GitHub Actions workflow is included. Changes
Sequence Diagram(s)Hacker-Typer Typing ModesequenceDiagram
participant User
participant VSCode
participant TextTypingService
participant Editor
User->>VSCode: Triggers insert/replace/patch with "hacker-typer" mode
VSCode->>TextTypingService: Call insert/replace/applyPatch
TextTypingService->>Editor: Prepare editor/session for hacker-typer
loop For each chunk
User->>VSCode: Press "next chunk" key/command
VSCode->>TextTypingService: COMMAND.hackerTyperNextChunk
TextTypingService->>Editor: Insert next chunk at cursor
end
TextTypingService->>VSCode: Resolve session, reset context/UI
API: List Demos EndpointsequenceDiagram
participant Client
participant DemoApi
participant DemoFileProvider
participant DemoStatusBar
participant DemoPanel
Client->>DemoApi: GET /api/demos
DemoApi->>DemoFileProvider: Get demo files
DemoFileProvider-->>DemoApi: Return demo file list
DemoApi->>DemoStatusBar: Get next demo
DemoStatusBar-->>DemoApi: Return next demo
DemoApi->>DemoPanel: Get current executing demo file
DemoPanel-->>DemoApi: Return current demo file
DemoApi-->>Client: Respond with demos, next, and current
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
View your CI Pipeline Execution ↗ for commit f0cc4cd
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit f2a446a
☁️ Nx Cloud last updated this comment at |
1 similar comment
|
View your CI Pipeline Execution ↗ for commit f2a446a
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This is a major release version 1.8.0 that introduces the hacker-typer typing mode and several other enhancements. The PR adds a new typing mode where content is inserted in chunks on each keystroke, similar to a hacker-typer effect, along with improvements to slide loading, terminal operations, and API endpoints.
- Added
hacker-typertyping mode with chunk-based content insertion for insert, replace, and patch actions - Added
openTerminalaction to open new terminals without executing commands - Improved slide loading mechanism with ready state notifications to prevent premature reveals
Reviewed Changes
Copilot reviewed 30 out of 33 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/TextTypingService.ts | Implements core hacker-typer functionality with session management and command registration |
| src/services/DemoRunner.ts | Integrates openTerminal action and fixes highlight positioning logic |
| src/services/TerminalService.ts | Refactors terminal creation into reusable openTerminal method |
| src/services/DemoStatusBar.ts | Adds hacker-typer mode status bar indicator |
| src/services/DemoApi.ts | Implements new /api/demos endpoint for retrieving demo information |
| src/models/Demos.ts | Updates type definitions to include hacker-typer mode and optional description |
| src/preview/components/ | Implements slide ready notifications to improve loading experience |
| package.json | Updates version and adds configuration options for hacker-typer chunk size |
| tests/TextTypingService.test.ts | Adds basic tests for the new typing mode |
Comments suppressed due to low confidence (1)
src/services/TextTypingService.ts:35
- The variable name 'i' is ambiguous in the context of a session object. Consider renaming it to 'currentIndex' or 'position' to better describe its purpose as the current position in the content being typed.
i: number;
…afer property access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/preview/components/MarkdownPreview.tsx (1)
156-158: Revert to single-line early return format.This formatting change contradicts previous feedback about maintaining consistency with typical early return patterns. The curly braces are unnecessary for a single statement.
Apply this diff to maintain consistency:
- if (!isZoomed || !ref.current) { - return; - } + if (!isZoomed || !ref.current) return;src/services/DemoApi.ts (1)
118-121: The nested ternary and map operations make this code hard to readConsider extracting the demo mapping logic into a separate function for better maintainability.
- const demoFilesFormatted = Object.entries(demoFiles).map(([filePath, file]) => ({ - filePath, - demos: DemoApi.mapDemos(file.demos) - })); + const demoFilesFormatted = DemoApi.formatDemoFiles(demoFiles);Add this private method:
private static formatDemoFiles(demoFiles: Record<string, any>): Array<{filePath: string, demos: {id: string, title: string}[]}> { return Object.entries(demoFiles).map(([filePath, file]) => ({ filePath, demos: DemoApi.mapDemos(file.demos) })); }
🧹 Nitpick comments (3)
tests/TextTypingService.test.ts (1)
1-40: LGTM! Consider expanding test coverage for runtime behavior.The test structure is well-organized with proper mocking of VSCode dependencies. The type-level tests correctly verify that the new
'hacker-typer'mode is accepted by theInsertTypingModetype.Consider adding tests for the actual runtime behavior of the hacker-typer mode, such as:
- Testing the command registration and execution
- Verifying the chunked typing functionality
- Testing integration with the TextTypingService
+ describe('Hacker-typer runtime behavior', () => { + it('should register hacker-typer command', () => { + // Test command registration + }); + + it('should execute chunked typing when triggered', () => { + // Test actual typing behavior + }); + });package.json (1)
801-803: Remove empty configuration objectsThe empty
nxanddependenciesobjects are unnecessary. If there are no dependencies or nx configuration, these properties should be omitted entirely.- }, - "nx": {}, - "dependencies": { } }src/services/TextTypingService.ts (1)
30-39: Consider extracting the hacker-typer session type into an interface.For better type safety and maintainability, consider defining a proper interface for the session state.
Add this interface definition before the class:
interface HackerTyperSession { editor: TextEditor; content: string; chunkSize: number; currentPos: Position; i: number; token: CancellationToken | undefined; done: boolean; resolve: undefined | ((value?: unknown) => void); }Then update the property declaration:
- private static crntHackerTyperSession: { - editor: TextEditor; - content: string; - chunkSize: number; - currentPos: Position; - i: number; - token: CancellationToken | undefined; - done: boolean; - resolve: undefined | ((value?: unknown) => void); - } | undefined; + private static crntHackerTyperSession: HackerTyperSession | undefined;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (32)
.github/copilot-instructions.md(1 hunks).github/workflows/pr-checks.yml(1 hunks).gitignore(1 hunks)CHANGELOG.md(1 hunks)assets/api/index.html(3 hunks)nx.json(1 hunks)package.json(9 hunks)src/constants/Command.ts(1 hunks)src/constants/Config.ts(1 hunks)src/constants/ContextKeys.ts(1 hunks)src/constants/WebViewMessages.ts(1 hunks)src/extension.ts(2 hunks)src/models/Action.ts(1 hunks)src/models/Demos.ts(1 hunks)src/panels/DemoPanel.ts(1 hunks)src/preview/Preview.ts(1 hunks)src/preview/components/App.tsx(1 hunks)src/preview/components/ImagePreview.tsx(2 hunks)src/preview/components/Markdown.tsx(1 hunks)src/preview/components/MarkdownPreview.tsx(1 hunks)src/preview/webcomponents/List.tsx(1 hunks)src/services/ChatActionsService.ts(1 hunks)src/services/DecoratorService.ts(9 hunks)src/services/DemoApi.ts(4 hunks)src/services/DemoRunner.ts(28 hunks)src/services/DemoStatusBar.ts(2 hunks)src/services/TerminalService.ts(2 hunks)src/services/TextTypingService.ts(8 hunks)src/utils/getActionOptions.ts(1 hunks)src/utils/getActionTemplate.ts(1 hunks)tests/TextTypingService.test.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
src/panels/DemoPanel.ts (1)
src/models/DemoFileCache.ts (1)
DemoFileCache(3-7)
src/preview/components/Markdown.tsx (1)
src/constants/WebViewMessages.ts (1)
WebViewMessages(1-51)
src/extension.ts (1)
src/services/TextTypingService.ts (1)
TextTypingService(29-851)
src/preview/components/ImagePreview.tsx (1)
src/constants/WebViewMessages.ts (1)
WebViewMessages(1-51)
src/preview/components/App.tsx (1)
src/constants/WebViewMessages.ts (1)
WebViewMessages(1-51)
tests/TextTypingService.test.ts (2)
src/services/Extension.ts (1)
Extension(4-118)src/models/Demos.ts (1)
InsertTypingMode(6-6)
src/preview/Preview.ts (1)
src/constants/WebViewMessages.ts (1)
WebViewMessages(1-51)
src/services/DecoratorService.ts (2)
src/services/Extension.ts (1)
Extension(4-118)src/constants/Config.ts (1)
Config(1-44)
src/services/TextTypingService.ts (8)
src/services/Extension.ts (2)
subscriptions(59-61)Extension(4-118)src/models/Subscription.ts (1)
Subscription(1-1)src/constants/Command.ts (1)
COMMAND(3-51)src/utils/setContext.ts (1)
setContext(3-5)src/constants/ContextKeys.ts (1)
ContextKeys(1-13)src/services/DemoStatusBar.ts (1)
DemoStatusBar(11-393)src/utils/writeFile.ts (1)
writeFile(5-13)src/constants/Config.ts (1)
Config(1-44)
🔇 Additional comments (35)
tsconfig.json (1)
2-14: Formatting-only changes look goodPure re-indentation and removal of the dangling comma; no semantic impact on the compiler options. 👍
src/models/Action.ts (1)
41-41: LGTM! Clean enum addition.The new
OpenTerminalaction is properly placed in the Terminal section and follows the established naming conventions consistently..gitignore (1)
9-10: LGTM! Standard Nx workspace exclusions.The addition of
.nx/cacheand.nx/workspace-dataentries is correct and necessary for proper Nx workspace integration.src/panels/DemoPanel.ts (1)
26-28: LGTM! Well-implemented getter for API access.The getter properly exposes read-only access to the current executing demo file state while maintaining encapsulation. This supports the new
/api/demosendpoint functionality.src/constants/Config.ts (1)
26-26: LGTM! Well-integrated configuration addition.The new
hackerTyperChunkSizeconfiguration key is properly placed in theinsertsection and follows established naming conventions. This correctly supports the new hacker-typer typing mode functionality.src/constants/Command.ts (1)
49-51: Command entry looks goodThe identifier follows the existing naming convention and is correctly namespaced, so command registration should work as expected.
src/extension.ts (2)
17-18: Ensure disposables fromTextTypingServiceare returnedImport looks fine; just double-check that
TextTypingService.registerCommands()internally pushes its disposables intoExtension.getInstance(context).subscriptions(same pattern as other services).
If not, commands may outlive the extension deactivation.
42-43: LGTM – service correctly wired into activation
This adds the typing-mode commands to the normal activation path without side effects.src/constants/ContextKeys.ts (1)
12-13: Context key addition approved
isHackerTyperis clearly named and follows the existing pattern; no further issues spotted.src/constants/WebViewMessages.ts (1)
30-31: Message constant fits the existing protocol
slideReadyis placed in the correct direction (toVscode) and should keep message routing explicit. No conflicts detected.src/utils/getActionOptions.ts (1)
196-199: LGTM! Clean integration of the new OpenTerminal action.The new action option follows the established pattern and is appropriately placed within the terminal actions section.
src/preview/Preview.ts (1)
217-219: Excellent improvement to slide reveal timing!Moving the
Preview.reveal()call to theslideReadymessage handler ensures the preview is only revealed after the webview content is fully prepared, improving the user experience by preventing premature reveals.src/preview/components/ImagePreview.tsx (2)
1-1: LGTM! Proper imports for the slideReady messaging system.The imports are correctly added to support the new slide readiness notification feature.
Also applies to: 3-3
12-16: Clean implementation of slide readiness signaling!The useEffect properly triggers the slideReady message when the fileUri changes, ensuring the preview system knows when the image content is loaded. The dependency array is correct and the comment clearly explains the intent.
src/utils/getActionTemplate.ts (1)
222-227: LGTM! Proper template structure for OpenTerminal action.The new action template follows the established pattern with appropriate default values. The
terminalIdproperty as an empty string provides a clear placeholder for user configuration.src/models/Demos.ts (2)
6-6: LGTM! Proper extension of typing modes.Adding 'hacker-typer' to the
InsertTypingModeunion type correctly supports the new interactive typing feature introduced in this release.
14-14: LGTM! Appropriate change to optional description.Making the
descriptionproperty optional inDemoFilealigns with PR objective #211 and provides more flexibility in demo configuration.src/preview/components/App.tsx (1)
34-38: LGTM! Clean implementation following existing patterns.The new message handler for
triggerUpdateis well-implemented with proper payload validation. The code correctly checks for payload existence before accessing the nestedfileUriStringproperty, and the logic is consistent with the existingupdateFileUrihandler.assets/api/index.html (2)
75-167: Excellent comprehensive API documentation.The new "List Demos" endpoint documentation is thorough and well-structured, including:
- Clear endpoint description and purpose
- Complete JSON response format with sample data
- Detailed field descriptions for all response properties
- Multiple usage examples with copy functionality
The documentation follows the established pattern and provides developers with all necessary information to use the new
/api/demosendpoint effectively.
396-396: Good formatting improvements.The added whitespace in the JavaScript functions improves code readability without affecting functionality.
Also applies to: 413-413
src/services/ChatActionsService.ts (1)
17-20: LGTM! Consistent implementation of optional message support.The changes correctly implement the optional
messageproperty for GitHub Copilot chat actions. All three methods (askChat,editChat,agentChat) now consistently:
- Always execute the chat command regardless of message presence
- Use
step.message || ''to provide a fallback empty string for the query parameter- Maintain consistent behavior across all chat action types
This aligns perfectly with the PR objective of making the message property optional for chat actions.
Also applies to: 25-28, 33-36
src/services/DemoRunner.ts (3)
77-112: Excellent formatting and documentation improvements.The extensive formatting enhancements significantly improve code readability through:
- Consistent spacing and blank line usage
- Enhanced method documentation with clear parameter descriptions
- Better visual separation between logical code blocks
The code structure and core logic remain unchanged, maintaining existing functionality while improving maintainability.
Also applies to: 133-162, 169-200, 208-227, 229-244, 246-326, 328-408, 410-510, 512-584, 586-640, 642-704, 706-814, 816-877, 879-906, 908-957, 959-978, 990-1027, 1029-1069, 1071-1078, 1080-1161
888-896: Good fix for highlight positioning behavior.The added logic correctly addresses the issue with highlighting when both line number and character positioning are used. The regex pattern
/^\d+,\d+:\d+,\d+$/properly identifies coordinate-format positions (e.g., "0,0:0,0") and forceshighlightWholeLinetofalsefor precise character-level highlighting.This fix aligns with the PR objective of resolving highlight behavior issues when using specific position formats.
722-726: New terminal action support added.The implementation correctly handles the new
OpenTerminalaction by callingTerminalService.openTerminal()with the specified terminal ID. This supports the new terminal opening capability mentioned in the PR objectives.src/preview/webcomponents/List.tsx (1)
143-156: LGTM! Improved HTML rendering for list items.The switch from
React.cloneElementtoReact.createElementwithdangerouslySetInnerHTMLensures that HTML content within list items is properly rendered instead of being escaped as plain text. This aligns well with markdown processing where list items may contain formatted HTML.The attribute mapping is preserved correctly, and this change supports the broader slide rendering improvements mentioned in the PR objectives.
CHANGELOG.md (1)
3-24: Well-documented release notes for version 1.8.0.The changelog entry comprehensively documents all the major features and fixes included in this release. Each item is properly linked to its corresponding GitHub issue, and the descriptions clearly explain the changes. This aligns perfectly with the PR objectives and follows good changelog practices.
.github/workflows/pr-checks.yml (1)
1-36: Well-structured CI workflow for PR validation.The workflow follows GitHub Actions best practices with proper permissions, efficient caching, and Nx integration for affected project testing. The use of
--legacy-peer-depson Line 27 suggests some dependency conflicts but is a common approach for managing peer dependency issues.The workflow appropriately runs formatting checks, linting, and testing on affected projects, which should provide good CI coverage while maintaining performance.
nx.json (1)
1-58: Comprehensive Nx workspace configuration.The configuration properly sets up caching for all build targets, includes appropriate output directories, and integrates Nx Cloud for distributed caching. The named inputs correctly reference the GitHub workflow file for dependency tracking, and the included scripts cover all necessary compilation and development tasks.
This should provide significant build performance improvements through intelligent caching and incremental builds.
src/services/DemoStatusBar.ts (2)
17-17: Property added for hacker mode status bar item.The new private static property follows the established pattern for other status bar items in the class.
204-225: Well-implemented hacker mode status bar integration.The
toggleHackerModemethod follows the established patterns in the class:
- Lazy initialization of the status bar item
- Consistent theming with error background for visual prominence
- Proper command, tooltip, and icon configuration
- Clean show/hide logic based on the boolean parameter
The implementation integrates seamlessly with the existing status bar management system and should provide clear visual feedback when hacker typer mode is active.
src/services/DecoratorService.ts (1)
173-181: Good improvement to method flexibilityThe addition of the optional
textEditorparameter and the early return pattern when no editor is available improves the method's flexibility and robustness..github/copilot-instructions.md (1)
1-64: Excellent documentation additionThis comprehensive documentation file provides clear instructions and context for developers working on the vscode-demo-time project. It covers all essential aspects including architecture, patterns, workflows, and integration points.
src/services/TextTypingService.ts (3)
41-110: Well-implemented command registration with proper session management.The implementation correctly addresses previous review feedback:
- No type safety bypassing with
undefined as any- Variable shadowing avoided by using
idxinstead ofi- Comprehensive session state validation and cleanup
186-189: Consistent integration of hacker-typer mode.The hacker-typer mode is properly integrated into existing insert and replace methods following the established pattern for other typing modes.
Also applies to: 213-218, 314-317, 347-352
820-827: Clean implementation of configuration retrieval.The helper method properly retrieves the chunk size with a sensible default value.
…rSession resolve property
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/services/TerminalService.ts (1)
157-169: Well-implemented terminal management method.The
openTerminalmethod correctly centralizes terminal creation and management. The logic properly handles terminal reuse, creation, and showing.Note: The method is marked as
asyncbut doesn't perform any asynchronous operations. Consider making it synchronous unless future async operations are planned:- public static async openTerminal(terminalId?: string): Promise<Terminal> { + public static openTerminal(terminalId?: string): Terminal {However, keeping it async maintains consistency with other service methods and allows for future async operations if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/extension.ts(2 hunks)src/preview/components/Markdown.tsx(1 hunks)src/services/TerminalService.ts(5 hunks)src/services/TextTypingService.ts(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/preview/components/Markdown.tsx
- src/extension.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/services/TextTypingService.ts (8)
src/services/Extension.ts (2)
subscriptions(59-61)Extension(4-118)src/models/Subscription.ts (1)
Subscription(1-1)src/constants/Command.ts (1)
COMMAND(3-51)src/utils/setContext.ts (1)
setContext(3-5)src/constants/ContextKeys.ts (1)
ContextKeys(1-13)src/services/DemoStatusBar.ts (1)
DemoStatusBar(11-393)src/utils/writeFile.ts (1)
writeFile(5-13)src/constants/Config.ts (1)
Config(1-44)
🔇 Additional comments (6)
src/services/TextTypingService.ts (1)
1-853: Code implementation looks good!The hacker-typer mode implementation is well-structured and properly integrated into the existing typing service. The code correctly:
- Manages session state for chunk-based typing
- Handles all text manipulation scenarios (insert, replace, patch)
- Updates UI elements appropriately (status bar, context keys)
- Addresses previous review concerns about variable shadowing and type casting
src/services/TerminalService.ts (5)
1-1: LGTM!The addition of
Disposableto the import statement is correct and necessary for the new disposable management functionality.
13-14: LGTM!The new static property for storing the terminal close event disposable is properly typed and follows good practices for resource management.
15-26: Excellent fix for the memory leak issue!This implementation directly addresses the memory leak concern from previous reviews by centralizing the terminal close event handler registration. The event is now registered once instead of every time a terminal is created, preventing handler accumulation.
42-42: Good refactoring to centralize terminal management.Delegating terminal creation and retrieval to the dedicated
openTerminalmethod improves code organization and maintainability.
118-127: Proper resource management implementation.The new
disposemethod correctly implements the disposable pattern by cleaning up the event handler disposable before disposing all terminals. The use of optional chaining for the disposable is appropriate.
|



switching between demo and slides
openTerminalaction toopen a the terminal
hacker-typertyping mode to theinsert,replace, andpatchactionsmessagepropertyoptional for the GitHub Copilot chat actions
refactored to have intellisense per action on which properties are required and optional
pathproperty is notrequired anymore for the
unselectactionoptional on the demo root
in the
dt-listcomponentcharacter positioning is used
/api/demosendpoint toretrieve the list of demos in the workspace
Summary by CodeRabbit