feat: clearer Arguments editor with typed-together detection and one-click fix
- Label now explains the model with an example: a flag and its value go
on separate lines. Placeholder shows the common uv pattern.
- split_suspicious_args() flags lines that contain whitespace plus a
dash-prefixed token ('--directory /path') — legit single args with
spaces ('My Documents') are never touched. Quotes are respected.
- The editor shows a warning under the args box with a 'Fix: split onto
separate lines' button; the fix goes through the undo stack.
This commit is contained in:
@@ -360,14 +360,30 @@ class ServerEditor(QFrame):
|
|||||||
self.command.setPlaceholderText("npx, uvx, node, python3, or a full path")
|
self.command.setPlaceholderText("npx, uvx, node, python3, or a full path")
|
||||||
self.command.textChanged.connect(self._emit)
|
self.command.textChanged.connect(self._emit)
|
||||||
v.addWidget(self.command)
|
v.addWidget(self.command)
|
||||||
v.addWidget(self._lbl("Arguments (one per line)"))
|
args_lbl = self._lbl(
|
||||||
self.args = ArgsEdit()
|
"Arguments — each line is one argument. A flag and its value go on "
|
||||||
self.args.setPlaceholderText(
|
"separate lines, e.g. “--directory” on line 1, “/path/to/server” on line 2."
|
||||||
"-y\n@modelcontextprotocol/server-filesystem\n/Users/you/Documents"
|
|
||||||
)
|
)
|
||||||
|
args_lbl.setWordWrap(True)
|
||||||
|
v.addWidget(args_lbl)
|
||||||
|
self.args = ArgsEdit()
|
||||||
|
self.args.setPlaceholderText("--directory\n/path/to/your/server\nrun\nmain.py")
|
||||||
self.args.setMinimumHeight(70)
|
self.args.setMinimumHeight(70)
|
||||||
self.args.textChanged.connect(self._emit)
|
self.args.textChanged.connect(self._emit)
|
||||||
v.addWidget(self.args, 1)
|
v.addWidget(self.args, 1)
|
||||||
|
|
||||||
|
# Shown when a line looks like several arguments typed together.
|
||||||
|
self.args_warn = QLabel("")
|
||||||
|
self.args_warn.setStyleSheet(f"color: {WARN};")
|
||||||
|
self.args_warn.setWordWrap(True)
|
||||||
|
self.args_warn.hide()
|
||||||
|
self.args_fix_btn = QPushButton("Fix: split onto separate lines")
|
||||||
|
self.args_fix_btn.clicked.connect(self._fix_args)
|
||||||
|
self.args_fix_btn.hide()
|
||||||
|
warn_row = QHBoxLayout()
|
||||||
|
warn_row.addWidget(self.args_warn, 1)
|
||||||
|
warn_row.addWidget(self.args_fix_btn)
|
||||||
|
v.addLayout(warn_row)
|
||||||
v.addWidget(self._lbl("Environment variables"))
|
v.addWidget(self._lbl("Environment variables"))
|
||||||
self.env = KeyValueTable(
|
self.env = KeyValueTable(
|
||||||
"Variable", "Value", on_change=self._emit, before_change=self._before_change
|
"Variable", "Value", on_change=self._emit, before_change=self._before_change
|
||||||
@@ -411,6 +427,8 @@ class ServerEditor(QFrame):
|
|||||||
self.details_btn.setChecked(False)
|
self.details_btn.setChecked(False)
|
||||||
self.diag_text.clear()
|
self.diag_text.clear()
|
||||||
self._set_dep({"status": "unknown", "label": "—"})
|
self._set_dep({"status": "unknown", "label": "—"})
|
||||||
|
self.args_warn.hide()
|
||||||
|
self.args_fix_btn.hide()
|
||||||
self._loading = False
|
self._loading = False
|
||||||
return
|
return
|
||||||
self.setEnabled(True)
|
self.setEnabled(True)
|
||||||
@@ -440,6 +458,7 @@ class ServerEditor(QFrame):
|
|||||||
self.env.load(d.get("env", {}))
|
self.env.load(d.get("env", {}))
|
||||||
self._loading = False
|
self._loading = False
|
||||||
self.refresh_dependency(auto_open=True)
|
self.refresh_dependency(auto_open=True)
|
||||||
|
self._check_args()
|
||||||
|
|
||||||
def dump_data(self) -> dict:
|
def dump_data(self) -> dict:
|
||||||
"""Build the server dict from the form, preserving unknown fields."""
|
"""Build the server dict from the form, preserving unknown fields."""
|
||||||
@@ -485,6 +504,32 @@ class ServerEditor(QFrame):
|
|||||||
if not self._loading and self._on_change:
|
if not self._loading and self._on_change:
|
||||||
self._on_change()
|
self._on_change()
|
||||||
self.refresh_dependency(auto_open=False)
|
self.refresh_dependency(auto_open=False)
|
||||||
|
self._check_args()
|
||||||
|
|
||||||
|
# --- args sanity check ------------------------------------------------ #
|
||||||
|
def _current_arg_lines(self) -> list[str]:
|
||||||
|
return [ln for ln in self.args.toPlainText().splitlines() if ln.strip() != ""]
|
||||||
|
|
||||||
|
def _check_args(self):
|
||||||
|
"""Warn (and offer a fix) when a line looks like several args in one."""
|
||||||
|
if self.type.currentIndex() != 0: # stdio page only
|
||||||
|
self.args_warn.hide()
|
||||||
|
self.args_fix_btn.hide()
|
||||||
|
return
|
||||||
|
_, notes = core.split_suspicious_args(self._current_arg_lines())
|
||||||
|
if notes:
|
||||||
|
self.args_warn.setText("⚠ " + "\n".join(notes))
|
||||||
|
self.args_warn.show()
|
||||||
|
self.args_fix_btn.show()
|
||||||
|
else:
|
||||||
|
self.args_warn.hide()
|
||||||
|
self.args_fix_btn.hide()
|
||||||
|
|
||||||
|
def _fix_args(self):
|
||||||
|
if self._before_change:
|
||||||
|
self._before_change()
|
||||||
|
fixed, _ = core.split_suspicious_args(self._current_arg_lines())
|
||||||
|
self.args.setPlainText("\n".join(fixed)) # triggers _emit -> recheck
|
||||||
|
|
||||||
# --- dependency ------------------------------------------------------ #
|
# --- dependency ------------------------------------------------------ #
|
||||||
def refresh_dependency(self, auto_open=False):
|
def refresh_dependency(self, auto_open=False):
|
||||||
|
|||||||
+36
@@ -580,6 +580,42 @@ def parse_pasted_json(text: str) -> dict[str, dict]:
|
|||||||
return servers
|
return servers
|
||||||
|
|
||||||
|
|
||||||
|
def split_suspicious_args(args: list[str]) -> tuple[list[str], list[str]]:
|
||||||
|
"""
|
||||||
|
Detect the classic argument-entry mistake: several argv tokens typed on one
|
||||||
|
line ("--directory /path/to/server"). An entry is only flagged when it
|
||||||
|
contains whitespace AND at least one whitespace-separated token starts with
|
||||||
|
"-" — so legitimate single arguments with spaces ("My Project Notes",
|
||||||
|
"/Users/me/My Documents") are never touched.
|
||||||
|
|
||||||
|
Returns (fixed_args, notes). notes is empty when nothing was suspicious;
|
||||||
|
otherwise it describes each split so a UI can show the proposed fix.
|
||||||
|
Quotes are respected when splitting ('--name "My Server"' becomes
|
||||||
|
['--name', 'My Server']).
|
||||||
|
"""
|
||||||
|
import shlex
|
||||||
|
|
||||||
|
out: list[str] = []
|
||||||
|
notes: list[str] = []
|
||||||
|
for a in args:
|
||||||
|
if isinstance(a, str) and _looks_like_multiple_args(a):
|
||||||
|
try:
|
||||||
|
parts = shlex.split(a)
|
||||||
|
except ValueError: # unbalanced quotes — fall back to plain split
|
||||||
|
parts = a.split()
|
||||||
|
if len(parts) > 1:
|
||||||
|
out.extend(parts)
|
||||||
|
notes.append(f"“{a}” looks like {len(parts)} arguments — split onto separate lines")
|
||||||
|
continue
|
||||||
|
out.append(a)
|
||||||
|
return out, notes
|
||||||
|
|
||||||
|
|
||||||
|
def _looks_like_multiple_args(a: str) -> bool:
|
||||||
|
toks = a.split()
|
||||||
|
return len(toks) > 1 and any(t.startswith("-") for t in toks)
|
||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# Validation
|
# Validation
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
|
|||||||
@@ -172,6 +172,40 @@ def test_kitchen_sink():
|
|||||||
assert len(notes) >= 4
|
assert len(notes) >= 4
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# Suspicious-args detection (several argv tokens typed on one line)
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_split_flag_and_value_on_one_line():
|
||||||
|
fixed, notes = c.split_suspicious_args(["--directory /path/to/server", "run", "main.py"])
|
||||||
|
assert fixed == ["--directory", "/path/to/server", "run", "main.py"]
|
||||||
|
assert len(notes) == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_split_full_option_string():
|
||||||
|
fixed, notes = c.split_suspicious_args(["-y @pkg/server --port 8080"])
|
||||||
|
assert fixed == ["-y", "@pkg/server", "--port", "8080"]
|
||||||
|
assert notes
|
||||||
|
|
||||||
|
|
||||||
|
def test_split_respects_quotes():
|
||||||
|
fixed, _ = c.split_suspicious_args(['--name "My Server"'])
|
||||||
|
assert fixed == ["--name", "My Server"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_split_leaves_legit_spaces_alone():
|
||||||
|
args = ["/Users/me/My Documents", "hello world", "run"]
|
||||||
|
fixed, notes = c.split_suspicious_args(args)
|
||||||
|
assert fixed == args
|
||||||
|
assert notes == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_split_leaves_clean_args_alone():
|
||||||
|
args = ["--directory", "/path/to/server", "run", "main.py"]
|
||||||
|
fixed, notes = c.split_suspicious_args(args)
|
||||||
|
assert fixed == args
|
||||||
|
assert notes == []
|
||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# Config-file repair (load-time)
|
# Config-file repair (load-time)
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
|
|||||||
Reference in New Issue
Block a user