Skip to content

enhance: support trigger shortcut without enter event#457

Merged
9aoy merged 1 commit intomainfrom
keypress-event
Aug 8, 2025
Merged

enhance: support trigger shortcut without enter event#457
9aoy merged 1 commit intomainfrom
keypress-event

Conversation

@9aoy
Copy link
Copy Markdown
Collaborator

@9aoy 9aoy commented Aug 8, 2025

Summary

support trigger shortcut without enter, use keypress instead of line event.

Related Links

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copilot AI review requested due to automatic review settings August 8, 2025 07:31
@netlify
Copy link
Copy Markdown

netlify bot commented Aug 8, 2025

Deploy Preview for rstest-dev ready!

Name Link
🔨 Latest commit 1ee4ac7
🔍 Latest deploy log https://app.netlify.com/projects/rstest-dev/deploys/6895a7d03301350008fc1166
😎 Deploy Preview https://deploy-preview-457--rstest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown
Contributor

Copilot AI left a 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 PR enhances the CLI shortcuts functionality by enabling immediate trigger of shortcuts without requiring the Enter key. The change switches from using 'line' events to 'keypress' events for a more responsive user experience.

Key changes:

  • Replace line-based input handling with raw keypress detection
  • Update user-facing messages to remove "+ enter" from shortcut descriptions
  • Implement proper raw mode handling with cleanup functions

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
packages/core/src/core/runTests.ts Updates CLI shortcut help messages to remove "+ enter" references
packages/core/src/core/cliShortcuts.ts Replaces readline 'line' event with raw keypress handling for immediate shortcut responses

}
};

process.stdin.on('keypress', handleKeypress);
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'keypress' event is not available by default in Node.js. You need to import and use the 'keypress' module or handle raw stdin data directly. Consider using process.stdin.on('data', ...) instead or import a keypress library.

Suggested change
process.stdin.on('keypress', handleKeypress);
process.stdin.on('data', handleKeypress);

Copilot uses AI. Check for mistakes.
process.stdin.resume();
process.stdin.setEncoding('utf8');

const handleKeypress = (
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key parameter with properties like name, ctrl, meta, and shift suggests this is expecting a parsed key object, but raw stdin data will only provide the raw string/buffer. The key parameter structure doesn't match what raw stdin provides.

Copilot uses AI. Check for mistakes.
key: { name: string; ctrl: boolean; meta: boolean; shift: boolean },
) => {
// Handle Ctrl+C
if (key.ctrl && key.name === 'c') {
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using raw mode with stdin data events, Ctrl+C will be received as a raw character (\u0003) rather than a parsed key object. This condition will never be true with the current implementation.

Suggested change
if (key.ctrl && key.name === 'c') {
if (str === '\u0003') {

Copilot uses AI. Check for mistakes.
@9aoy 9aoy merged commit 9e9db38 into main Aug 8, 2025
17 checks passed
@9aoy 9aoy deleted the keypress-event branch August 8, 2025 07:42
@9aoy 9aoy mentioned this pull request Aug 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants