diff --git a/phabfive/setup.py b/phabfive/setup.py index 77cb6f5..fac8f6c 100644 --- a/phabfive/setup.py +++ b/phabfive/setup.py @@ -6,10 +6,11 @@ import os import re import sys +import webbrowser from phabricator import Phabricator, APIError +from InquirerPy import inquirer as inq from rich.console import Console -from rich.prompt import Prompt, Confirm from phabfive.constants import VALIDATORS @@ -17,6 +18,31 @@ log = logging.getLogger(__name__) +def _has_display() -> bool: + """Check if we can reasonably auto-open a browser. + + Returns False in environments where webbrowser.open() would either fail + or open a text-mode browser in the terminal: + + - WSL2: Python's webbrowser module finds text browsers (lynx, w3m) instead + of the Windows browser. Detected via /proc/sys/kernel/osrelease containing + "microsoft". + - Docker containers: No display server available. Detected via the + /.dockerenv sentinel file. + - Podman / systemd-nspawn containers: No display server available. Detected + via the "container" environment variable set by these runtimes. + """ + try: + with open("/proc/sys/kernel/osrelease") as f: + if "microsoft" in f.read().lower(): + return False + except OSError: + pass + if os.path.exists("/.dockerenv") or os.environ.get("container"): + return False + return True + + class SetupWizard: """Interactive setup wizard for phabfive configuration.""" @@ -30,35 +56,58 @@ def __init__(self): def _check_existing_config(self) -> bool: """Check if there's an existing working configuration. - If a working config exists, warn the user and ask for confirmation. + Reads ~/.arcrc directly to avoid triggering the interactive server + selector in Phabfive(). Tests each host silently and warns if any + connection works. Returns: bool: True to proceed with setup, False to abort """ try: - from phabfive.core import Phabfive - - # Try to create a Phabfive instance - this validates config and connection - phabfive = Phabfive() - whoami = phabfive.phab.user.whoami() - username = whoami.get("userName", "unknown") - - self.console.print( - "\n[yellow]Warning:[/yellow] A working configuration already exists." - ) - self.console.print( - f"Currently connected to [bold]{phabfive.conf['PHAB_URL']}[/bold] " - f"as [bold]{username}[/bold].\n" - ) - - if not Confirm.ask("Do you want to reconfigure phabfive?", default=False): - self.console.print("Setup cancelled.\n") - return False - + if not os.path.exists(self.CONFIG_PATH): + return True + + with open(self.CONFIG_PATH, "r") as f: + arcrc = json.load(f) + + hosts = arcrc.get("hosts", {}) + if not hosts: + return True + + # Try each host silently to find a working connection + for host_url, host_data in hosts.items(): + token = host_data.get("token") + if not token: + continue + try: + phab = Phabricator(host=host_url, token=token) + phab.update_interfaces() + whoami = phab.user.whoami() + username = whoami.get("userName", "unknown") + + self.console.print( + "\n[yellow]Warning:[/yellow] A working configuration already exists." + ) + self.console.print( + f"Currently connected to [bold]{host_url}[/bold] " + f"as [bold]{username}[/bold].\n" + ) + + if not inq.confirm( + message="Do you want to reconfigure phabfive?", default=False + ).execute(): + self.console.print("Setup cancelled.\n") + return False + + return True + except Exception: + continue + + # No working host found - proceed with setup return True except Exception: - # No valid config or connection failed - proceed with setup + # Cannot read .arcrc or other error - proceed with setup return True def run(self) -> bool: @@ -94,8 +143,8 @@ def run(self) -> bool: def _print_header(self): """Print the setup wizard header.""" - self.console.print("\n[bold]Phabfive Setup[/bold]") - self.console.print("=" * 40) + self.console.print() + self.console.rule("[bold]Phabfive Setup[/bold]") self.console.print( "\nThis wizard will configure phabfive to connect to your " "Phabricator/Phorge instance.\n" @@ -103,12 +152,12 @@ def _print_header(self): def _prompt_url(self) -> bool: """Prompt for and validate the Phabricator URL.""" - self.console.print("[bold][1/3] Phabricator URL[/bold]") + self.console.rule("[bold][1/3] Phabricator URL[/bold]") while True: - url = Prompt.ask( - "Enter your Phabricator URL (e.g., https://phorge.example.com)" - ) + url = inq.text( + message="Enter your Phabricator URL (e.g., phorge.example.com):" + ).execute() if not url: self.console.print("[red]URL cannot be empty[/red]") @@ -126,30 +175,34 @@ def _prompt_url(self) -> bool: continue self.phab_url = normalized - self.console.print(f"[green]> Using API endpoint: {normalized}[/green]\n") + self.console.print(f" [green]✓[/green] API endpoint: {normalized}\n") return True def _prompt_token(self) -> bool: """Prompt for and validate the API token.""" - self.console.print("[bold][2/3] API Token[/bold]") + self.console.rule("[bold][2/3] API Token[/bold]") - # Extract base URL for settings link from urllib.parse import urlparse parsed = urlparse(self.phab_url) base_url = f"{parsed.scheme}://{parsed.netloc}" + token_url = f"{base_url}/settings/panel/apitokens/" - self.console.print("To create an API token:") - self.console.print( - f" 1. Go to {base_url}/settings/user/YOUR_USERNAME/page/apitokens/" - ) - self.console.print(' 2. Click "Generate API Token"') - self.console.print(" 3. Copy the token (starts with 'cli-')\n") - - from InquirerPy import inquirer as inq + if _has_display(): + self.console.print( + f"Opening [bold]{token_url}[/bold] in your browser to generate a token.\n" + ) + webbrowser.open(token_url) + self.console.print( + "[dim]If the browser didn't open, visit the URL above manually.[/dim]\n" + ) + else: + self.console.print( + f"Open this URL in your browser to generate a token:\n\n [bold]{token_url}[/bold]\n" + ) while True: - token = inq.secret(message="Enter your API token:").execute() + token = inq.secret(message="Paste your API token here:").execute() if not token: self.console.print("[red]Token cannot be empty[/red]") @@ -169,25 +222,23 @@ def _prompt_token(self) -> bool: def _verify_connection(self) -> bool: """Verify the connection to Phabricator.""" - self.console.print("[bold][3/3] Verifying connection...[/bold]") + self.console.rule("[bold][3/3] Verify Connection[/bold]") try: - phab = Phabricator(host=self.phab_url, token=self.phab_token) - phab.update_interfaces() - whoami = phab.user.whoami() + with self.console.status("Verifying connection..."): + phab = Phabricator(host=self.phab_url, token=self.phab_token) + phab.update_interfaces() + whoami = phab.user.whoami() username = whoami.get("userName", "unknown") realname = whoami.get("realName", "") if realname: self.console.print( - f"[green]> Connected successfully as: " - f"{username} ({realname})[/green]\n" + f" [green]✓[/green] Connected as: {username} ({realname})\n" ) else: - self.console.print( - f"[green]> Connected successfully as: {username}[/green]\n" - ) + self.console.print(f" [green]✓[/green] Connected as: {username}\n") return True @@ -195,7 +246,9 @@ def _verify_connection(self) -> bool: self.console.print(f"[red]> Connection failed: {e}[/red]") self.console.print("[red]Please check your URL and token.[/red]\n") - if Confirm.ask("Would you like to try again?", default=True): + if inq.confirm( + message="Would you like to try again?", default=True + ).execute(): return ( self._prompt_url() and self._prompt_token() @@ -241,10 +294,14 @@ def _save_config(self) -> bool: def _print_success(self): """Print success message.""" - self.console.print(f"[green]Configuration saved to {self.CONFIG_PATH}[/green]") + self.console.print() + self.console.rule("[bold green]Setup Complete[/bold green]") + self.console.print( + f"\n [green]✓[/green] Configuration saved to {self.CONFIG_PATH}" + ) if os.name != "nt": self.console.print( - "[green]File permissions set to 0600 (owner read/write only)[/green]" + " [green]✓[/green] File permissions set to 0600 (owner read/write only)" ) self.console.print("\n[bold]Example commands:[/bold]") self.console.print(" phabfive user whoami") @@ -252,6 +309,8 @@ def _print_success(self): def _normalize_url(self, url: str) -> str: """Normalize URL to end with /api/.""" + if not url.startswith(("http://", "https://")): + url = "https://" + url url = url.rstrip("/") if not url.endswith("/api"): url += "/api" @@ -312,16 +371,18 @@ def _setup_arcconfig(console) -> bool: except (json.JSONDecodeError, IOError): pass - if not Confirm.ask("Do you want to overwrite it?", default=False): + if not inq.confirm( + message="Do you want to overwrite it?", default=False + ).execute(): return False - console.print("[bold]Create .arcconfig[/bold]") + console.rule("[bold]Create .arcconfig[/bold]") console.print(f"This will create .arcconfig in: {git_root}\n") while True: - url = Prompt.ask( - "Enter your Phabricator URL (e.g., https://phorge.example.com)" - ) + url = inq.text( + message="Enter your Phabricator URL (e.g., https://phorge.example.com):" + ).execute() if not url: console.print("[red]URL cannot be empty[/red]") @@ -333,7 +394,7 @@ def _setup_arcconfig(console) -> bool: if url.endswith("/api"): url = url[:-4].rstrip("/") - console.print(f"[green]> Using URL: {url}[/green]\n") + console.print(f" [green]✓[/green] Using URL: {url}\n") break try: @@ -342,7 +403,7 @@ def _setup_arcconfig(console) -> bool: json.dump(arcconfig_data, f, indent=2) f.write("\n") - console.print(f"[green].arcconfig created at {arcconfig_path}[/green]") + console.print(f" [green]✓[/green] .arcconfig created at {arcconfig_path}") console.print("[dim]Remember to commit .arcconfig to your repository.[/dim]\n") return True @@ -376,13 +437,15 @@ def offer_setup_on_error(error_message: str) -> bool: is_url_error = "PHAB_URL" in error_message and "PHAB_TOKEN" not in error_message if is_url_error: - if Confirm.ask( - "Would you like to create .arcconfig for this repository?", + if inq.confirm( + message="Would you like to create .arcconfig for this repository?", default=True, - ): + ).execute(): return _setup_arcconfig(console) else: - if Confirm.ask("Would you like to run interactive setup now?", default=True): + if inq.confirm( + message="Would you like to run interactive setup now?", default=True + ).execute(): wizard = SetupWizard() return wizard.run() diff --git a/tests/test_setup.py b/tests/test_setup.py index 5c8fcba..de98de7 100644 --- a/tests/test_setup.py +++ b/tests/test_setup.py @@ -84,9 +84,11 @@ def test_non_interactive_returns_false(self): def test_interactive_declines_returns_false(self): """Test that declining interactive setup returns False.""" + mock_confirm = mock.MagicMock() + mock_confirm.return_value.execute.return_value = False with ( mock.patch("sys.stdin.isatty", return_value=True), - mock.patch("phabfive.setup.Confirm.ask", return_value=False), + mock.patch("phabfive.setup.inq.confirm", mock_confirm), mock.patch("phabfive.setup.Console"), ): result = offer_setup_on_error("Test error") @@ -94,31 +96,31 @@ def test_interactive_declines_returns_false(self): def test_phab_url_error_offers_arcconfig(self): """Test that PHAB_URL error offers .arcconfig setup, not full wizard.""" + mock_confirm = mock.MagicMock() + mock_confirm.return_value.execute.return_value = False with ( mock.patch("sys.stdin.isatty", return_value=True), - mock.patch( - "phabfive.setup.Confirm.ask", return_value=False - ) as mock_confirm, + mock.patch("phabfive.setup.inq.confirm", mock_confirm), mock.patch("phabfive.setup.Console"), ): offer_setup_on_error("PHAB_URL is not configured") # Should ask about .arcconfig, not generic setup - call_args = mock_confirm.call_args[0][0] + call_args = mock_confirm.call_args[1]["message"] assert ".arcconfig" in call_args def test_phab_token_error_offers_full_setup(self): """Test that PHAB_TOKEN error offers full setup wizard.""" + mock_confirm = mock.MagicMock() + mock_confirm.return_value.execute.return_value = False with ( mock.patch("sys.stdin.isatty", return_value=True), - mock.patch( - "phabfive.setup.Confirm.ask", return_value=False - ) as mock_confirm, + mock.patch("phabfive.setup.inq.confirm", mock_confirm), mock.patch("phabfive.setup.Console"), ): offer_setup_on_error("PHAB_TOKEN is not configured") - call_args = mock_confirm.call_args[0][0] + call_args = mock_confirm.call_args[1]["message"] assert "interactive setup" in call_args @@ -151,12 +153,11 @@ def test_setup_arcconfig_creates_file(self, tmp_path): console = mock.MagicMock() + mock_text = mock.MagicMock() + mock_text.return_value.execute.return_value = "https://phorge.example.com" with ( mock.patch("os.getcwd", return_value=str(tmp_path)), - mock.patch( - "phabfive.setup.Prompt.ask", - return_value="https://phorge.example.com", - ), + mock.patch("phabfive.setup.inq.text", mock_text), ): result = _setup_arcconfig(console) @@ -179,12 +180,11 @@ def test_setup_arcconfig_strips_api_suffix(self, tmp_path): console = mock.MagicMock() + mock_text = mock.MagicMock() + mock_text.return_value.execute.return_value = "https://phorge.example.com/api/" with ( mock.patch("os.getcwd", return_value=str(tmp_path)), - mock.patch( - "phabfive.setup.Prompt.ask", - return_value="https://phorge.example.com/api/", - ), + mock.patch("phabfive.setup.inq.text", mock_text), ): _setup_arcconfig(console) @@ -357,7 +357,10 @@ def test_verify_connection_api_error(self): with ( mock.patch("phabfive.setup.Phabricator") as mock_phab, mock.patch("phabfive.setup.Console"), - mock.patch("phabfive.setup.Confirm.ask", return_value=False), + mock.patch( + "phabfive.setup.inq.confirm", + lambda **kw: mock.MagicMock(execute=mock.MagicMock(return_value=False)), + ), ): mock_instance = mock_phab.return_value mock_instance.update_interfaces.return_value = None