From 4c14c1bafa09f8142590db9244d82084969aa0aa Mon Sep 17 00:00:00 2001 From: User Date: Thu, 10 Jul 2025 03:30:54 -0700 Subject: [PATCH 1/2] feat: change mcpm edit arguments from CSV to space-separated format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Updated argument parsing from comma-separated to space-separated - Changed prompts from "Arguments (comma-separated)" to "Arguments (space-separated)" - Modified parsing logic from .split(",") to .split() - Updated table display format to show space-separated arguments - Updated tests to expect space-separated output Fixes #212 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/mcpm/commands/edit.py | 20 +++++++++----------- tests/test_edit.py | 2 +- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/mcpm/commands/edit.py b/src/mcpm/commands/edit.py index b1293ea..97b15ad 100644 --- a/src/mcpm/commands/edit.py +++ b/src/mcpm/commands/edit.py @@ -80,7 +80,7 @@ def edit(server_name, new, editor): if isinstance(server_config, STDIOServerConfig): table.add_row("Command", server_config.command) - table.add_row("Arguments", ", ".join(server_config.args) if server_config.args else "[dim]None[/]") + table.add_row("Arguments", " ".join(server_config.args) if server_config.args else "[dim]None[/]") table.add_row( "Environment", ", ".join(f"{k}={v}" for k, v in server_config.env.items()) if server_config.env else "[dim]None[/]", @@ -187,10 +187,10 @@ def interactive_server_edit(server_config) -> Optional[Dict[str, Any]]: keybindings={"interrupt": [{"key": "escape"}]}, ).execute() - # Arguments as comma-separated string - current_args = ", ".join(server_config.args) if server_config.args else "" + # Arguments as space-separated string + current_args = " ".join(server_config.args) if server_config.args else "" answers["args"] = inquirer.text( - message="Arguments (comma-separated):", + message="Arguments (space-separated):", default=current_args, instruction="(Leave empty for no arguments)", keybindings={"interrupt": [{"key": "escape"}]}, @@ -237,7 +237,7 @@ def interactive_server_edit(server_config) -> Optional[Dict[str, Any]]: if isinstance(server_config, STDIOServerConfig): console.print(f"Command: [cyan]{server_config.command}[/] → [cyan]{answers['command']}[/]") - new_args = [arg.strip() for arg in answers["args"].split(",") if arg.strip()] if answers["args"] else [] + new_args = answers["args"].split() if answers["args"] else [] console.print(f"Arguments: [cyan]{server_config.args}[/] → [cyan]{new_args}[/]") new_env = {} @@ -298,7 +298,7 @@ def apply_interactive_changes(server_config, interactive_result): # Parse arguments if answers["args"].strip(): - server_config.args = [arg.strip() for arg in answers["args"].split(",") if arg.strip()] + server_config.args = answers["args"].split() else: server_config.args = [] @@ -381,9 +381,7 @@ def _create_new_server(): server_config = STDIOServerConfig( name=server_name, command=result["answers"]["command"], - args=[arg.strip() for arg in result["answers"]["args"].split(",") if arg.strip()] - if result["answers"]["args"] - else [], + args=result["answers"]["args"].split() if result["answers"]["args"] else [], env={}, ) @@ -462,7 +460,7 @@ def _interactive_new_server_form() -> Optional[Dict[str, Any]]: ).execute() answers["args"] = inquirer.text( - message="Arguments (comma-separated):", + message="Arguments (space-separated):", instruction="(Leave empty for no arguments)", keybindings={"interrupt": [{"key": "escape"}]}, ).execute() @@ -497,7 +495,7 @@ def _interactive_new_server_form() -> Optional[Dict[str, Any]]: if answers["type"] == "stdio": console.print(f"Command: [cyan]{answers['command']}[/]") - new_args = [arg.strip() for arg in answers["args"].split(",") if arg.strip()] if answers["args"] else [] + new_args = answers["args"].split() if answers["args"] else [] console.print(f"Arguments: [cyan]{new_args}[/]") new_env = {} diff --git a/tests/test_edit.py b/tests/test_edit.py index bb27a26..6c496e5 100644 --- a/tests/test_edit.py +++ b/tests/test_edit.py @@ -44,7 +44,7 @@ def test_edit_server_interactive_fallback(monkeypatch): assert result.exit_code == 0 # CliRunner may not properly handle our return codes assert "Current Configuration for 'test-server'" in result.output assert "test-cmd" in result.output - assert "arg1, arg2" in result.output + assert "arg1 arg2" in result.output assert "KEY=value" in result.output assert "test-profile" in result.output assert "Interactive editing not available" in result.output From 97f2973e0d980ec74a523cecfd06b75aa702411f Mon Sep 17 00:00:00 2001 From: User Date: Thu, 10 Jul 2025 03:40:41 -0700 Subject: [PATCH 2/2] fix: use shlex.split() for proper argument parsing with spaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Import shlex module for proper shell-like argument parsing - Replace .split() with shlex.split() for argument parsing - Update prompts to indicate quote support for arguments with spaces - Add tests for arguments containing spaces - Add unit tests to verify shlex parsing behavior This addresses the review feedback about handling arguments with spaces correctly. Arguments like "path with spaces" are now properly parsed as single arguments. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/mcpm/commands/edit.py | 17 ++++++------- tests/test_edit.py | 50 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/src/mcpm/commands/edit.py b/src/mcpm/commands/edit.py index 97b15ad..878817b 100644 --- a/src/mcpm/commands/edit.py +++ b/src/mcpm/commands/edit.py @@ -3,6 +3,7 @@ """ import os +import shlex import subprocess import sys from typing import Any, Dict, Optional @@ -190,9 +191,9 @@ def interactive_server_edit(server_config) -> Optional[Dict[str, Any]]: # Arguments as space-separated string current_args = " ".join(server_config.args) if server_config.args else "" answers["args"] = inquirer.text( - message="Arguments (space-separated):", + message="Arguments (space-separated, quotes supported):", default=current_args, - instruction="(Leave empty for no arguments)", + instruction="(Leave empty for no arguments, use quotes for args with spaces)", keybindings={"interrupt": [{"key": "escape"}]}, ).execute() @@ -237,7 +238,7 @@ def interactive_server_edit(server_config) -> Optional[Dict[str, Any]]: if isinstance(server_config, STDIOServerConfig): console.print(f"Command: [cyan]{server_config.command}[/] → [cyan]{answers['command']}[/]") - new_args = answers["args"].split() if answers["args"] else [] + new_args = shlex.split(answers["args"]) if answers["args"] else [] console.print(f"Arguments: [cyan]{server_config.args}[/] → [cyan]{new_args}[/]") new_env = {} @@ -298,7 +299,7 @@ def apply_interactive_changes(server_config, interactive_result): # Parse arguments if answers["args"].strip(): - server_config.args = answers["args"].split() + server_config.args = shlex.split(answers["args"]) else: server_config.args = [] @@ -381,7 +382,7 @@ def _create_new_server(): server_config = STDIOServerConfig( name=server_name, command=result["answers"]["command"], - args=result["answers"]["args"].split() if result["answers"]["args"] else [], + args=shlex.split(result["answers"]["args"]) if result["answers"]["args"] else [], env={}, ) @@ -460,8 +461,8 @@ def _interactive_new_server_form() -> Optional[Dict[str, Any]]: ).execute() answers["args"] = inquirer.text( - message="Arguments (space-separated):", - instruction="(Leave empty for no arguments)", + message="Arguments (space-separated, quotes supported):", + instruction="(Leave empty for no arguments, use quotes for args with spaces)", keybindings={"interrupt": [{"key": "escape"}]}, ).execute() @@ -495,7 +496,7 @@ def _interactive_new_server_form() -> Optional[Dict[str, Any]]: if answers["type"] == "stdio": console.print(f"Command: [cyan]{answers['command']}[/]") - new_args = answers["args"].split() if answers["args"] else [] + new_args = shlex.split(answers["args"]) if answers["args"] else [] console.print(f"Arguments: [cyan]{new_args}[/]") new_env = {} diff --git a/tests/test_edit.py b/tests/test_edit.py index 6c496e5..535dadb 100644 --- a/tests/test_edit.py +++ b/tests/test_edit.py @@ -2,6 +2,7 @@ Tests for the edit command """ +import shlex from unittest.mock import Mock from click.testing import CliRunner @@ -51,6 +52,32 @@ def test_edit_server_interactive_fallback(monkeypatch): assert "This command requires a terminal for interactive input" in result.output +def test_edit_server_with_spaces_in_args(monkeypatch): + """Test display of arguments with spaces in the fallback view.""" + test_server = STDIOServerConfig( + name="test-server", + command="test-cmd", + args=["arg with spaces", "another arg", "--flag=value with spaces"], + env={"KEY": "value"}, + profile_tags=["test-profile"], + ) + + mock_global_config = Mock() + mock_global_config.get_server.return_value = test_server + monkeypatch.setattr("mcpm.commands.edit.global_config_manager", mock_global_config) + + runner = CliRunner() + result = runner.invoke(edit, ["test-server"]) + + # In test environment, interactive mode falls back and shows message + assert result.exit_code == 0 + assert "Current Configuration for 'test-server'" in result.output + assert "test-cmd" in result.output + # Check that arguments with spaces are displayed correctly + assert "arg with spaces another arg --flag=value with spaces" in result.output + assert "Interactive editing not available" in result.output + + def test_edit_command_help(): """Test the edit command help output.""" runner = CliRunner() @@ -94,3 +121,26 @@ def test_edit_editor_flag(monkeypatch): # Verify subprocess.run was called with correct arguments mock_subprocess.assert_called_once_with(["open", "/tmp/test_servers.json"]) + + +def test_shlex_argument_parsing(): + """Test that shlex correctly parses arguments with spaces.""" + # Test basic space-separated arguments + result = shlex.split("arg1 arg2 arg3") + assert result == ["arg1", "arg2", "arg3"] + + # Test quoted arguments with spaces + result = shlex.split('arg1 "arg with spaces" arg3') + assert result == ["arg1", "arg with spaces", "arg3"] + + # Test mixed quotes + result = shlex.split("arg1 'arg with spaces' --flag=\"value with spaces\"") + assert result == ["arg1", "arg with spaces", "--flag=value with spaces"] + + # Test empty string + result = shlex.split("") + assert result == [] + + # Test single argument with spaces + result = shlex.split('"single arg with spaces"') + assert result == ["single arg with spaces"]