Add support for activating tabs with window focus#103
Add support for activating tabs with window focus#103prasmussen merged 6 commits intoprasmussen:masterfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for activating tabs while also bringing their containing window to the front, addressing scenarios where Chrome windows may be in different Spaces or fullscreen mode. The implementation adds a new --focus flag to the existing activate command.
- Added
activateTabAndFocusmethod that combines tab activation with window focusing - Implemented accessibility-based window focusing with retry logic for robust cross-Space navigation
- Updated documentation to reflect the new
--focusfunctionality
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| chrome-cli/main.m | Added command line argument for activate -t <id> --focus |
| chrome-cli/App.h | Added method declaration for activateTabAndFocus |
| chrome-cli/App.m | Core implementation with accessibility APIs and retry logic |
| chrome-cli.xcodeproj/project.pbxproj | Added ApplicationServices and AppKit framework dependencies |
| README.md | Updated documentation with new --focus flag examples |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } else { | ||
| } | ||
| } | ||
|
|
||
| chromeTab *tab = [self findTab:tabId inWindow:window]; | ||
| if (!tab) { | ||
| return; | ||
| if (!done) { | ||
| chromeTab *tabAny = [self findTab:tabId]; | ||
| chromeWindow *winAny = tabAny ? [self findWindowWithTab:tabAny] : nil; | ||
| if (tabAny && winAny) { | ||
| [self setTabActive:tabAny inWindow:winAny]; | ||
| } else { | ||
| } | ||
| } |
There was a problem hiding this comment.
Empty else blocks at lines 439 and 448 make the code unclear. Consider removing these empty blocks or adding comments explaining why no action is needed in these cases.
| if (!focused) { | ||
| } |
There was a problem hiding this comment.
Empty conditional block serves no purpose. Consider removing this if statement or adding appropriate error handling/logging.
| if (!focused) { | |
| } | |
| // If not focused, no action is taken. |
| chromeWindow *aw = [self activeWindow]; | ||
| if (aw && [aw.id isEqualToString:w.id]) { | ||
| return YES; | ||
| } else { |
There was a problem hiding this comment.
Empty else block serves no purpose. Consider removing this empty block or adding a comment explaining why no action is needed when the window focus verification fails.
| } else { |
| - (void)activateTabAndFocus:(Arguments *)args { | ||
| [self activateTab:args]; | ||
|
|
||
| NSString *rawId = [args asString:@"id"]; | ||
| if (!rawId || rawId.length == 0) { | ||
| return; | ||
| } | ||
|
|
||
| // Determine tabId and attempt to focus the window containing it with retries | ||
| NSInteger tabId = 0; | ||
| NSRange sep = [rawId rangeOfString:@":"]; | ||
| if (sep.location != NSNotFound) { | ||
| NSString *tabStr = [rawId substringFromIndex:sep.location + 1]; | ||
| tabId = [tabStr integerValue]; | ||
| } else { | ||
| tabId = [rawId integerValue]; | ||
| } |
There was a problem hiding this comment.
The tab ID parsing logic is duplicated from the activateTab: method. Consider extracting this into a helper method to avoid code duplication and improve maintainability.
2 participants
Just added a quick --focus on the same
activatemethod that I added profiles before. Feel free to any questions