feat: add Microsoft Edge browser support#6
Conversation
📝 WalkthroughWalkthroughThe PR extends chrome-devtools-cli to support Edge browsers alongside Chrome. CLI arguments, browser resolution logic, and documentation are updated to recognize Edge channels ( ChangesChrome and Edge Browser Support Expansion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request adds support for Microsoft Edge browser channels (edge, edge-beta, edge-canary, edge-dev) across macOS, Linux, and Windows, updating the documentation, CLI help text, and user data directory resolution logic accordingly. It also documents Windows daemon details (TCP localhost IPC). The review feedback suggests converting the channel string to lowercase before matching to ensure case-insensitive channel selection.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| fn default_chrome_user_data_dir(channel: &str) -> Result<PathBuf> { | ||
| #[cfg(target_os = "macos")] | ||
| { | ||
| let home = dirs::home_dir().ok_or_else(|| anyhow!("Cannot determine home directory"))?; | ||
| let base = home.join("Library/Application Support/Google"); | ||
| let dir = match channel { |
There was a problem hiding this comment.
The channel parameter is matched against exact lowercase string literals (e.g., "edge", "stable"). If a user passes a channel with different casing (e.g., --channel Edge or --channel Stable), the match will fail with an Unknown browser channel error. Converting the channel string to lowercase before matching makes the CLI more robust and user-friendly.
| fn default_chrome_user_data_dir(channel: &str) -> Result<PathBuf> { | |
| #[cfg(target_os = "macos")] | |
| { | |
| let home = dirs::home_dir().ok_or_else(|| anyhow!("Cannot determine home directory"))?; | |
| let base = home.join("Library/Application Support/Google"); | |
| let dir = match channel { | |
| fn default_chrome_user_data_dir(channel: &str) -> Result<PathBuf> { | |
| let channel_lower = channel.to_lowercase(); | |
| let channel = channel_lower.as_str(); | |
| #[cfg(target_os = "macos")] | |
| { | |
| let home = dirs::home_dir().ok_or_else(|| anyhow!("Cannot determine home directory"))?; | |
| let dir = match channel { |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 181-183: Update the README to stop hardcoding `/tmp` for Unix temp
paths and instead state that IPC socket and PID file are placed in the system
temporary directory (the implementation uses std::env::temp_dir() in
src/protocol.rs), e.g. "system temp directory (returned by
std::env::temp_dir())" for Unix, while keeping the Windows %TEMP% references;
adjust the two bullets for IPC (macOS/Linux) and PID file (Unix) to reflect
"system temp-dir" rather than `/tmp`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 424fa9c6-8a81-4340-87f4-855c218881e5
📒 Files selected for processing (4)
README.mdskill/chrome-devtools/SKILL.mdsrc/browser.rssrc/lib.rs
| - **IPC (macOS/Linux)**: Unix socket at `/tmp/chrome-devtools-daemon.sock` | ||
| - **IPC (Windows)**: TCP on `127.0.0.1` with address stored in `%TEMP%\chrome-devtools-daemon.addr` | ||
| - **PID file**: `/tmp/chrome-devtools-daemon.pid` (Unix) or `%TEMP%\chrome-devtools-daemon.pid` (Windows) |
There was a problem hiding this comment.
Daemon path docs should not hardcode /tmp on Unix.
The implementation uses std::env::temp_dir() (src/protocol.rs), so Unix paths are not guaranteed to be /tmp/.... Please document these as system temp-dir paths to avoid debugging confusion.
Suggested doc patch
-- **IPC (macOS/Linux)**: Unix socket at `/tmp/chrome-devtools-daemon.sock`
-- **PID file**: `/tmp/chrome-devtools-daemon.pid` (Unix) or `%TEMP%\chrome-devtools-daemon.pid` (Windows)
+- **IPC (macOS/Linux)**: Unix socket in the system temp directory (e.g. `/tmp/chrome-devtools-daemon.sock`)
+- **PID file**: In the system temp directory (e.g. `/tmp/chrome-devtools-daemon.pid` on many Unix systems) or `%TEMP%\chrome-devtools-daemon.pid` (Windows)📝 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.
| - **IPC (macOS/Linux)**: Unix socket at `/tmp/chrome-devtools-daemon.sock` | |
| - **IPC (Windows)**: TCP on `127.0.0.1` with address stored in `%TEMP%\chrome-devtools-daemon.addr` | |
| - **PID file**: `/tmp/chrome-devtools-daemon.pid` (Unix) or `%TEMP%\chrome-devtools-daemon.pid` (Windows) | |
| - **IPC (macOS/Linux)**: Unix socket in the system temp directory (e.g. `/tmp/chrome-devtools-daemon.sock`) | |
| - **IPC (Windows)**: TCP on `127.0.0.1` with address stored in `%TEMP%\chrome-devtools-daemon.addr` | |
| - **PID file**: In the system temp directory (e.g. `/tmp/chrome-devtools-daemon.pid` on many Unix systems) or `%TEMP%\chrome-devtools-daemon.pid` (Windows) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 181 - 183, Update the README to stop hardcoding
`/tmp` for Unix temp paths and instead state that IPC socket and PID file are
placed in the system temporary directory (the implementation uses
std::env::temp_dir() in src/protocol.rs), e.g. "system temp directory (returned
by std::env::temp_dir())" for Unix, while keeping the Windows %TEMP% references;
adjust the two bullets for IPC (macOS/Linux) and PID file (Unix) to reflect
"system temp-dir" rather than `/tmp`.
|
@freebattle would u like to address the reviews from gemini and code rabbit above |
Summary by CodeRabbit
New Features
--channel edgeoption with variants (edge-beta, edge-canary, edge-dev) for targeting Edge instances.Documentation