mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
Merge pull request #13641 from NousResearch/bb/tui-at-folder-filter
fix(tui): @folder: / @file: completions respect the explicit prefix
This commit is contained in:
commit
3e198f37c9
4 changed files with 224 additions and 14 deletions
|
|
@ -924,12 +924,22 @@ class SlashCommandCompleter(Completer):
|
|||
display_meta=meta,
|
||||
)
|
||||
|
||||
# If the user typed @file: or @folder:, delegate to path completions
|
||||
# If the user typed @file: / @folder: (or just @file / @folder with
|
||||
# no colon yet), delegate to path completions. Accepting the bare
|
||||
# form lets the picker surface directories as soon as the user has
|
||||
# typed `@folder`, without requiring them to first accept the static
|
||||
# `@folder:` hint and re-trigger completion.
|
||||
for prefix in ("@file:", "@folder:"):
|
||||
if word.startswith(prefix):
|
||||
path_part = word[len(prefix):] or "."
|
||||
bare = prefix[:-1]
|
||||
|
||||
if word == bare or word.startswith(prefix):
|
||||
want_dir = prefix == "@folder:"
|
||||
path_part = '' if word == bare else word[len(prefix):]
|
||||
expanded = os.path.expanduser(path_part)
|
||||
if expanded.endswith("/"):
|
||||
|
||||
if not expanded or expanded == ".":
|
||||
search_dir, match_prefix = ".", ""
|
||||
elif expanded.endswith("/"):
|
||||
search_dir, match_prefix = expanded, ""
|
||||
else:
|
||||
search_dir = os.path.dirname(expanded) or "."
|
||||
|
|
@ -945,15 +955,21 @@ class SlashCommandCompleter(Completer):
|
|||
for entry in sorted(entries):
|
||||
if match_prefix and not entry.lower().startswith(prefix_lower):
|
||||
continue
|
||||
if count >= limit:
|
||||
break
|
||||
full_path = os.path.join(search_dir, entry)
|
||||
is_dir = os.path.isdir(full_path)
|
||||
# `@folder:` must only surface directories; `@file:` only
|
||||
# regular files. Without this filter `@folder:` listed
|
||||
# every .env / .gitignore in the cwd, defeating the
|
||||
# explicit prefix and confusing users expecting a
|
||||
# directory picker.
|
||||
if want_dir != is_dir:
|
||||
continue
|
||||
if count >= limit:
|
||||
break
|
||||
display_path = os.path.relpath(full_path)
|
||||
suffix = "/" if is_dir else ""
|
||||
kind = "folder" if is_dir else "file"
|
||||
meta = "dir" if is_dir else _file_size_label(full_path)
|
||||
completion = f"@{kind}:{display_path}{suffix}"
|
||||
completion = f"{prefix}{display_path}{suffix}"
|
||||
yield Completion(
|
||||
completion,
|
||||
start_position=-len(word),
|
||||
|
|
|
|||
91
tests/gateway/test_complete_path_at_filter.py
Normal file
91
tests/gateway/test_complete_path_at_filter.py
Normal file
|
|
@ -0,0 +1,91 @@
|
|||
"""Regression tests for the TUI gateway's `complete.path` handler.
|
||||
|
||||
Reported during the TUI v2 blitz retest: typing `@folder:` (and `@folder`
|
||||
with no colon yet) still surfaced files alongside directories in the
|
||||
TUI composer, because the gateway-side completion lives in
|
||||
`tui_gateway/server.py` and was never touched by the earlier fix to
|
||||
`hermes_cli/commands.py`.
|
||||
|
||||
Covers:
|
||||
- `@folder:` only yields directories
|
||||
- `@file:` only yields regular files
|
||||
- Bare `@folder` / `@file` (no colon) lists cwd directly
|
||||
- Explicit prefix is preserved in the completion text
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from tui_gateway import server
|
||||
|
||||
|
||||
def _fixture(tmp_path: Path):
|
||||
(tmp_path / "readme.md").write_text("x")
|
||||
(tmp_path / ".env").write_text("x")
|
||||
(tmp_path / "src").mkdir()
|
||||
(tmp_path / "docs").mkdir()
|
||||
|
||||
|
||||
def _items(word: str):
|
||||
resp = server.handle_request({"id": "1", "method": "complete.path", "params": {"word": word}})
|
||||
|
||||
return [(it["text"], it["display"], it.get("meta", "")) for it in resp["result"]["items"]]
|
||||
|
||||
|
||||
def test_at_folder_colon_only_dirs(tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_fixture(tmp_path)
|
||||
|
||||
texts = [t for t, _, _ in _items("@folder:")]
|
||||
|
||||
assert all(t.startswith("@folder:") for t in texts), texts
|
||||
assert any(t == "@folder:src/" for t in texts)
|
||||
assert any(t == "@folder:docs/" for t in texts)
|
||||
assert not any(t == "@folder:readme.md" for t in texts)
|
||||
assert not any(t == "@folder:.env" for t in texts)
|
||||
|
||||
|
||||
def test_at_file_colon_only_files(tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_fixture(tmp_path)
|
||||
|
||||
texts = [t for t, _, _ in _items("@file:")]
|
||||
|
||||
assert all(t.startswith("@file:") for t in texts), texts
|
||||
assert any(t == "@file:readme.md" for t in texts)
|
||||
assert not any(t == "@file:src/" for t in texts)
|
||||
assert not any(t == "@file:docs/" for t in texts)
|
||||
|
||||
|
||||
def test_at_folder_bare_without_colon_lists_dirs(tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_fixture(tmp_path)
|
||||
|
||||
texts = [t for t, _, _ in _items("@folder")]
|
||||
|
||||
assert any(t == "@folder:src/" for t in texts), texts
|
||||
assert any(t == "@folder:docs/" for t in texts), texts
|
||||
assert not any(t == "@folder:readme.md" for t in texts)
|
||||
|
||||
|
||||
def test_at_file_bare_without_colon_lists_files(tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
_fixture(tmp_path)
|
||||
|
||||
texts = [t for t, _, _ in _items("@file")]
|
||||
|
||||
assert any(t == "@file:readme.md" for t in texts), texts
|
||||
assert not any(t == "@file:src/" for t in texts)
|
||||
|
||||
|
||||
def test_bare_at_still_shows_static_refs(tmp_path, monkeypatch):
|
||||
"""`@` alone should list the static references so users discover the
|
||||
available prefixes. (Unchanged behaviour; regression guard.)
|
||||
"""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
texts = [t for t, _, _ in _items("@")]
|
||||
|
||||
for expected in ("@diff", "@staged", "@file:", "@folder:", "@url:", "@git:"):
|
||||
assert expected in texts, f"missing static ref {expected!r} in {texts!r}"
|
||||
90
tests/hermes_cli/test_at_context_completion_filter.py
Normal file
90
tests/hermes_cli/test_at_context_completion_filter.py
Normal file
|
|
@ -0,0 +1,90 @@
|
|||
"""Regression test: `@folder:` completion must only surface directories and
|
||||
`@file:` must only surface regular files.
|
||||
|
||||
Reported during TUI v2 blitz testing: typing `@folder:` showed .dockerignore,
|
||||
.env, .gitignore, etc. alongside the actual directories because the path-
|
||||
completion branch yielded every entry regardless of the explicit prefix, and
|
||||
auto-switched the completion kind based on `is_dir`. That defeated the user's
|
||||
explicit choice and rendered the `@folder:` / `@file:` prefixes useless for
|
||||
filtering.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
from typing import Iterable
|
||||
|
||||
from hermes_cli.commands import SlashCommandCompleter
|
||||
|
||||
|
||||
def _run(tmp_path: Path, word: str) -> list[tuple[str, str]]:
|
||||
(tmp_path / "readme.md").write_text("x")
|
||||
(tmp_path / ".env").write_text("x")
|
||||
(tmp_path / "src").mkdir()
|
||||
(tmp_path / "docs").mkdir()
|
||||
|
||||
completer = SlashCommandCompleter.__new__(SlashCommandCompleter)
|
||||
completions: Iterable = completer._context_completions(word)
|
||||
|
||||
return [(c.text, c.display_meta) for c in completions if c.text.startswith(("@file:", "@folder:"))]
|
||||
|
||||
|
||||
def test_at_folder_only_yields_directories(tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
texts = [t for t, _ in _run(tmp_path, "@folder:")]
|
||||
|
||||
assert all(t.startswith("@folder:") for t in texts), texts
|
||||
assert any(t == "@folder:src/" for t in texts)
|
||||
assert any(t == "@folder:docs/" for t in texts)
|
||||
assert not any(t == "@folder:readme.md" for t in texts)
|
||||
assert not any(t == "@folder:.env" for t in texts)
|
||||
|
||||
|
||||
def test_at_file_only_yields_files(tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
texts = [t for t, _ in _run(tmp_path, "@file:")]
|
||||
|
||||
assert all(t.startswith("@file:") for t in texts), texts
|
||||
assert any(t == "@file:readme.md" for t in texts)
|
||||
assert any(t == "@file:.env" for t in texts)
|
||||
assert not any(t == "@file:src/" for t in texts)
|
||||
assert not any(t == "@file:docs/" for t in texts)
|
||||
|
||||
|
||||
def test_at_folder_preserves_prefix_on_empty_match(tmp_path, monkeypatch):
|
||||
"""User typed `@folder:` (no partial) — completion text must keep the
|
||||
`@folder:` prefix even though the previous implementation auto-rewrote
|
||||
it to `@file:` for non-dir entries.
|
||||
"""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
texts = [t for t, _ in _run(tmp_path, "@folder:")]
|
||||
|
||||
assert texts, "expected at least one directory completion"
|
||||
for t in texts:
|
||||
assert t.startswith("@folder:"), f"prefix leaked: {t}"
|
||||
|
||||
|
||||
def test_at_folder_bare_without_colon_lists_directories(tmp_path, monkeypatch):
|
||||
"""Typing `@folder` alone (no colon yet) should surface directories so
|
||||
users don't need to first accept the static `@folder:` hint before
|
||||
seeing what they're picking from.
|
||||
"""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
texts = [t for t, _ in _run(tmp_path, "@folder")]
|
||||
|
||||
assert any(t == "@folder:src/" for t in texts), texts
|
||||
assert any(t == "@folder:docs/" for t in texts), texts
|
||||
assert not any(t == "@folder:readme.md" for t in texts)
|
||||
|
||||
|
||||
def test_at_file_bare_without_colon_lists_files(tmp_path, monkeypatch):
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
texts = [t for t, _ in _run(tmp_path, "@file")]
|
||||
|
||||
assert any(t == "@file:readme.md" for t in texts), texts
|
||||
assert not any(t == "@file:src/" for t in texts)
|
||||
|
|
@ -2426,15 +2426,22 @@ def _(rid, params: dict) -> dict:
|
|||
]
|
||||
return _ok(rid, {"items": items})
|
||||
|
||||
if is_context and query.startswith(("file:", "folder:")):
|
||||
prefix_tag = query.split(":", 1)[0]
|
||||
path_part = query.split(":", 1)[1] or "."
|
||||
# Accept both `@folder:path` and the bare `@folder` form so the user
|
||||
# sees directory listings as soon as they finish typing the keyword,
|
||||
# without first accepting the static `@folder:` hint.
|
||||
if is_context and query in ("file", "folder"):
|
||||
prefix_tag, path_part = query, ""
|
||||
elif is_context and query.startswith(("file:", "folder:")):
|
||||
prefix_tag, _, tail = query.partition(":")
|
||||
path_part = tail
|
||||
else:
|
||||
prefix_tag = ""
|
||||
path_part = query if not is_context else query
|
||||
path_part = query if is_context else query
|
||||
|
||||
expanded = _normalize_completion_path(path_part)
|
||||
if expanded.endswith("/"):
|
||||
expanded = _normalize_completion_path(path_part) if path_part else "."
|
||||
if expanded == "." or not expanded:
|
||||
search_dir, match = ".", ""
|
||||
elif expanded.endswith("/"):
|
||||
search_dir, match = expanded, ""
|
||||
else:
|
||||
search_dir = os.path.dirname(expanded) or "."
|
||||
|
|
@ -2443,6 +2450,7 @@ def _(rid, params: dict) -> dict:
|
|||
if not os.path.isdir(search_dir):
|
||||
return _ok(rid, {"items": []})
|
||||
|
||||
want_dir = prefix_tag == "folder"
|
||||
match_lower = match.lower()
|
||||
for entry in sorted(os.listdir(search_dir)):
|
||||
if match and not entry.lower().startswith(match_lower):
|
||||
|
|
@ -2451,6 +2459,11 @@ def _(rid, params: dict) -> dict:
|
|||
continue
|
||||
full = os.path.join(search_dir, entry)
|
||||
is_dir = os.path.isdir(full)
|
||||
# Explicit `@folder:` / `@file:` — honour the user's filter. Skip
|
||||
# the opposite kind instead of auto-rewriting the completion tag,
|
||||
# which used to defeat the prefix and let `@folder:` list files.
|
||||
if prefix_tag and want_dir != is_dir:
|
||||
continue
|
||||
rel = os.path.relpath(full)
|
||||
suffix = "/" if is_dir else ""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue