-
-
Notifications
You must be signed in to change notification settings - Fork 36
DRAFT: feat: add terminal tracking support #44
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
base: master
Are you sure you want to change the base?
Conversation
- Add terminal event listeners (onDidChangeActiveTerminal, onDidOpenTerminal) - Track terminal activity with clean naming format: "Cursor/VSCode - Terminal - [project]" - Detect Cursor vs VSCode automatically using env.appName/appRoot - Update VS Code minimum version requirement to 1.29.0 for terminal API support - Add terminal-specific heartbeat data with terminal_name and terminal_pid fields - Maintain backward compatibility with existing editor tracking
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.
Caution
Changes requested ❌
Reviewed everything up to 20b3518 in 2 minutes and 27 seconds. Click for details.
- Reviewed
221lines of code in2files - Skipped
1files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. package.json:16
- Draft comment:
Engine version updated to ^1.29.0 is correct for Terminal API support. Consider verifying that the devDependencies 'vscode' version remains appropriate. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. src/extension.ts:152
- Draft comment:
Using a shared _lastHeartbeatTime for both terminal and editor events might throttle heartbeats unexpectedly. Consider separate rate limiting for terminal activity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The code shows that terminal and editor events are indeed using the same rate limiting mechanism through _lastHeartbeatTime. This could mean that a burst of terminal activity could prevent editor heartbeats from being sent, and vice versa. However, the current implementation seems intentional - it's a global rate limit across all activity types. The suggestion to separate them is a valid optimization but may not be critical. The comment assumes separate rate limiting would be better, but maybe the global rate limit is intentional to prevent overwhelming the server. The current behavior might be exactly what's desired. While the current implementation could be improved, the shared rate limiting might be an intentional design choice. The comment points out a real potential issue but doesn't make a strong enough case for why separate rate limiting is definitely better. Delete the comment. While it raises a valid point about potential optimization, it's more of a suggestion than a clear issue, and the current implementation might be intentionally designed this way.
3. src/extension.ts:185
- Draft comment:
The condition 'if (activeTerminal && !activeEditor)' defaults to editor context when both are active. Verify if this reflects the intended focus behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify if the behavior is intended, which violates the rule against asking for confirmation of intention. It does not provide a specific suggestion or point out a clear issue with the code.
4. src/extension.ts:250
- Draft comment:
Avoid logging app detection details unconditionally in production; consider gating the console.log with a debug flag. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. src/extension.ts:174
- Draft comment:
Destructuring the error in .catch(({ err })) may fail if the error object doesn't have an 'err' property. Use .catch(err => ...) to ensure robust error handling. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. src/extension.ts:330
- Draft comment:
There's a typo in the log message: '[AcitivtyWatch]' should be '[ActivityWatch]'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_zRb2RRTxtxtlHpbA
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
| if (workspace.workspaceFolders && workspace.workspaceFolders.length > 0) { | ||
| const workspaceFolder = workspace.workspaceFolders[0]; | ||
| // Extract just the folder name, not the full path | ||
| const pathParts = workspaceFolder.uri.path.split('/'); |
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.
Splitting workspaceFolder.uri.path on '/' may not handle different OS path separators correctly. Consider using path.basename for robust project name extraction.
Summary
Adds comprehensive terminal tracking to the ActivityWatch VS Code extension, enabling monitoring of terminal usage alongside existing editor activity tracking.
Changes
onDidChangeActiveTerminalandonDidOpenTerminalevent handlersCursor/VSCode - Terminal - [project]formatenv.appNameandenv.appRootterminal_nameandterminal_pidfieldsTerminal Activity Data Format
{ "language": "terminal", "file": "Cursor - Terminal - [project-name]", "project": "/path/to/project", "branch": "main", "terminal_name": "original terminal title", "terminal_pid": "12345" }Test Plan
Compatibility
Important
Adds terminal tracking to ActivityWatch VS Code extension, including terminal event listeners and data enhancements, while maintaining backward compatibility.
onDidChangeActiveTerminalandonDidOpenTerminalevent handlers inextension.ts.terminal_nameandterminal_pidfields.Cursor/VSCode - Terminal - [project].env.appNameandenv.appRootinextension.ts.package.jsonfor terminal API support._getCleanTerminalName(),_getProjectName(), and_getEditorName()functions inextension.ts.This description was created by
for 20b3518. You can customize this summary. It will automatically update as commits are pushed.