Skip to content

Issue #111: Open a site up in the browser on upping site#112

Merged
gthomas2 merged 3 commits into
mainfrom
issue_111_open_site_on_up
Mar 9, 2026
Merged

Issue #111: Open a site up in the browser on upping site#112
gthomas2 merged 3 commits into
mainfrom
issue_111_open_site_on_up

Conversation

@gthomas2

@gthomas2 gthomas2 commented Mar 9, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 implements the feature requested in Issue #111 to automatically open the site URL in the user's default browser when a site is brought up via mchef. It adds OS detection helpers, a cross-platform browser-open method in CliService, and integrates it into both the Main::up() flow and the Up command.

Changes:

  • Added isMac() and isLinux() static methods to the OS helper class for cross-platform OS detection.
  • Added openSite() method to CliService that opens a URL in the default browser, with CI mode detection and OS-specific command selection.
  • Integrated openSite() into both Main::up() and the Up command's execute() method to open the Moodle site after containers start.

Reviewed changes

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

File Description
src/Helpers/OS.php Added isMac() and isLinux() detection methods to complement existing isWindows().
src/Service/CliService.php Added openSite() method with CI-mode guard and cross-platform browser-open logic.
src/Service/Main.php Added CliService dependency and call to openSite() after the up() workflow completes.
src/Command/Up.php Added CliService dependency, wwwRoot notice, and openSite() call in the Up command.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Service/CliService.php Outdated
Comment thread src/Command/Up.php Outdated
Comment thread src/Service/CliService.php Outdated
Comment on lines +39 to +44
if (OS::isWindows()) {
$cmd = "start $url";
} elseif (OS::isMac()) {
$cmd = "open $url";
} elseif (OS::isLinux()) {
$cmd = "xdg-open $url";

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

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

On Windows, the start command interprets the first quoted argument as a window title, not a URL. If you add escapeshellarg() around $url (as recommended for security), start will treat the quoted URL as a window title and fail to open the browser. The conventional fix is to use start "" <url> to provide an empty title first. Even without escaping, this is a good practice to handle URLs that might contain spaces.

Suggested change
if (OS::isWindows()) {
$cmd = "start $url";
} elseif (OS::isMac()) {
$cmd = "open $url";
} elseif (OS::isLinux()) {
$cmd = "xdg-open $url";
$escapedUrl = escapeshellarg($url);
if (OS::isWindows()) {
$cmd = 'start "" ' . $escapedUrl;
} elseif (OS::isMac()) {
$cmd = 'open ' . $escapedUrl;
} elseif (OS::isLinux()) {
$cmd = 'xdg-open ' . $escapedUrl;

Copilot uses AI. Check for mistakes.
gthomas2 and others added 2 commits March 9, 2026 13:09
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@gthomas2 gthomas2 merged commit 57e7175 into main Mar 9, 2026
3 checks passed
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