Skip to content

Commit 223b7b5

Browse files
raifdmuellerclaude
andauthored
fix: CLI update warns on empty content + better --format position error (#256)
* fix: CLI update warns on empty content + better --format position error (#250, #176) - Issue #250: `update --content ""` now prints a warning to stderr ("Warning: Section content will be cleared.") instead of silently clearing the section. - Issue #176: When users place --format after the command (e.g. `dacli structure --format json`), the error message now explains that --format is a global option and must be placed before the command. Uses a GlobalOptionHintCommand class applied to all subcommands via AliasedGroup.command_class. - Bump version to 0.4.25. Fixes #250, Fixes #176 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: report circular includes as errors instead of orphaned files (#251) When AsciiDoc files include each other circularly, they were reported as "orphaned files" because the parser threw CircularIncludeError causing documents to not load. Now detect cycles in the include graph during index building and report them as "circular_include" errors in validation. Fixes #251 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: bump version to 0.4.26 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a837c18 commit 223b7b5

File tree

9 files changed

+433
-6
lines changed

9 files changed

+433
-6
lines changed

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "dacli"
3-
version = "0.4.25"
3+
version = "0.4.26"
44
description = "Documentation Access CLI - Navigate and query large documentation projects"
55
readme = "README.md"
66
license = { text = "MIT" }

src/dacli/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,4 @@
55
"""
66

77

8-
__version__ = "0.4.25"
8+
__version__ = "0.4.26"

src/dacli/cli.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,39 @@ def _get_section_append_line(
155155
COMMAND_TO_ALIAS = {v: k for k, v in COMMAND_ALIASES.items()}
156156

157157

158+
# Global options that users commonly misplace after the command (Issue #176)
159+
GLOBAL_OPTIONS = {
160+
"--format", "--pretty", "--verbose", "--docs-root",
161+
"--no-gitignore", "--include-hidden",
162+
}
163+
164+
165+
class GlobalOptionHintCommand(click.Command):
166+
"""A Click command that hints when users misplace global options."""
167+
168+
def parse_args(self, ctx, args):
169+
try:
170+
return super().parse_args(ctx, args)
171+
except click.UsageError as e:
172+
error_msg = str(e)
173+
if "No such option:" in error_msg:
174+
for opt in GLOBAL_OPTIONS:
175+
if opt in error_msg:
176+
raise click.UsageError(
177+
f"No such option: {opt}\n\n"
178+
f"Hint: --format, --pretty, and --verbose are global options.\n"
179+
f"Place them before the command: "
180+
f"dacli {opt} ... {ctx.info_name} ..."
181+
) from e
182+
raise
183+
184+
158185
class AliasedGroup(click.Group):
159186
"""A Click group that supports command aliases, typo suggestions, and grouped help."""
160187

188+
# Issue #176: Use GlobalOptionHintCommand for all subcommands by default
189+
command_class = GlobalOptionHintCommand
190+
161191
def get_command(self, ctx, cmd_name):
162192
"""Resolve command name, checking aliases first."""
163193
# Check if it's an alias
@@ -687,6 +717,10 @@ def update(ctx: CliContext, path: str, content: str, no_preserve_title: bool,
687717
else:
688718
processed_content = _process_escape_sequences(content)
689719

720+
# Issue #250: Warn when content is empty/whitespace-only
721+
if not processed_content.strip():
722+
click.echo("Warning: Section content will be cleared.", err=True)
723+
690724
result = service_update_section(
691725
index=ctx.index,
692726
file_handler=ctx.file_handler,

src/dacli/mcp_app.py

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
from fastmcp import FastMCP
2121

2222
from dacli import __version__
23-
from dacli.asciidoc_parser import AsciidocStructureParser
23+
from dacli.asciidoc_parser import AsciidocStructureParser, CircularIncludeError
2424
from dacli.file_handler import FileReadError, FileSystemHandler, FileWriteError
2525
from dacli.file_utils import find_doc_files
2626
from dacli.markdown_parser import MarkdownStructureParser
@@ -658,6 +658,51 @@ def _build_index(
658658
# Filter: only parse files that are NOT included by others (Issue #184)
659659
root_adoc_files = [f for f in all_adoc_files if f not in included_files]
660660

661+
# Issue #251: Detect circular includes in the include graph
662+
# Files that include each other circularly all end up in included_files
663+
# with none of them becoming root documents. Detect these cycles.
664+
circular_include_errors: list[dict] = []
665+
if all_adoc_files:
666+
include_graph: dict[Path, set[Path]] = {}
667+
for adoc_file in all_adoc_files:
668+
resolved = adoc_file.resolve()
669+
includes = AsciidocStructureParser.scan_includes(adoc_file)
670+
include_graph[resolved] = includes
671+
672+
circular_files: set[Path] = set()
673+
visited: set[Path] = set()
674+
in_stack: set[Path] = set()
675+
676+
def _find_cycles(node: Path, path_list: list[Path]) -> None:
677+
if node in in_stack:
678+
cycle_start = path_list.index(node)
679+
for f in path_list[cycle_start:]:
680+
circular_files.add(f)
681+
return
682+
if node in visited:
683+
return
684+
visited.add(node)
685+
in_stack.add(node)
686+
path_list.append(node)
687+
for neighbor in include_graph.get(node, set()):
688+
_find_cycles(neighbor, path_list)
689+
path_list.pop()
690+
in_stack.remove(node)
691+
692+
for adoc_file in all_adoc_files:
693+
_find_cycles(adoc_file.resolve(), [])
694+
695+
for circ_file in circular_files:
696+
message = (
697+
f"Circular include detected: {circ_file.name} "
698+
f"is part of an include cycle"
699+
)
700+
circular_include_errors.append({
701+
"file": circ_file,
702+
"include_chain": list(circular_files),
703+
"message": message,
704+
})
705+
661706
logger.info(
662707
f"Found {len(all_adoc_files)} AsciiDoc files, "
663708
f"{len(included_files)} included, "
@@ -669,6 +714,14 @@ def _build_index(
669714
try:
670715
doc = asciidoc_parser.parse_file(adoc_file)
671716
documents.append(doc)
717+
except CircularIncludeError as e:
718+
# Issue #251: Catch circular includes during parsing too
719+
logger.warning("Circular include in %s: %s", adoc_file, e)
720+
circular_include_errors.append({
721+
"file": adoc_file,
722+
"include_chain": e.include_chain,
723+
"message": str(e),
724+
})
672725
except Exception as e:
673726
# Log but continue with other files
674727
logger.warning("Failed to parse %s: %s", adoc_file, e)
@@ -695,4 +748,7 @@ def _build_index(
695748
for warning in warnings:
696749
logger.warning("Index: %s", warning)
697750

751+
# Issue #251: Store circular include errors on the index for validation
752+
index._circular_include_errors = circular_include_errors
753+
698754

src/dacli/services/validation_service.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,25 @@ def validate_structure(index: StructureIndex, docs_root: Path) -> dict:
3333
errors: list[dict] = []
3434
warnings: list[dict] = []
3535

36+
# Issue #251: Report circular include errors explicitly
37+
docs_root_resolved = docs_root.resolve()
38+
circular_files: set[Path] = set()
39+
for circ_error in index._circular_include_errors:
40+
file_path = circ_error["file"]
41+
try:
42+
rel_path = file_path.relative_to(docs_root_resolved)
43+
except ValueError:
44+
rel_path = file_path
45+
errors.append({
46+
"type": "circular_include",
47+
"path": str(rel_path),
48+
"message": circ_error["message"],
49+
})
50+
# Track all files involved in circular includes
51+
circular_files.add(file_path.resolve())
52+
for chain_path in circ_error["include_chain"]:
53+
circular_files.add(chain_path.resolve())
54+
3655
# Get all indexed files
3756
indexed_files = set(index._file_to_sections.keys())
3857

@@ -44,10 +63,10 @@ def validate_structure(index: StructureIndex, docs_root: Path) -> dict:
4463
all_doc_files.add(md_file.resolve())
4564

4665
# Check for orphaned files (files not indexed)
66+
# Issue #251: Exclude files involved in circular includes from orphaned detection
4767
indexed_resolved = {f.resolve() for f in indexed_files}
48-
docs_root_resolved = docs_root.resolve()
4968
for doc_file in all_doc_files:
50-
if doc_file not in indexed_resolved:
69+
if doc_file not in indexed_resolved and doc_file not in circular_files:
5170
try:
5271
rel_path = doc_file.relative_to(docs_root_resolved)
5372
except ValueError:

src/dacli/structure_index.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ def __init__(self) -> None:
6767
self._section_content: dict[str, str] = {} # Content for full-text search
6868
self._documents: list[Document] = []
6969
self._top_level_sections: list[Section] = []
70+
self._circular_include_errors: list[dict] = []
7071
self._index_ready: bool = False
7172

7273
def build_from_documents(self, documents: list[Document]) -> list[str]:
@@ -491,6 +492,7 @@ def clear(self) -> None:
491492
self._section_content.clear()
492493
self._documents.clear()
493494
self._top_level_sections.clear()
495+
self._circular_include_errors.clear()
494496
self._index_ready = False
495497

496498
def stats(self) -> dict:
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
"""Tests for Issue #251: Report circular includes explicitly instead of as orphaned files.
2+
3+
When two AsciiDoc files include each other circularly (A includes B, B includes A),
4+
the validation should report them as "circular_include" errors, not as "orphaned_file"
5+
warnings.
6+
"""
7+
8+
from pathlib import Path
9+
10+
import pytest
11+
from click.testing import CliRunner
12+
13+
from dacli.cli import cli
14+
from dacli.mcp_app import create_mcp_server
15+
16+
17+
@pytest.fixture
18+
def temp_circular_include(tmp_path: Path) -> Path:
19+
"""Create a temporary directory with two files that include each other."""
20+
file_a = tmp_path / "file_a.adoc"
21+
file_b = tmp_path / "file_b.adoc"
22+
23+
file_a.write_text(
24+
"""= File A
25+
26+
Some content in A.
27+
28+
include::file_b.adoc[]
29+
""",
30+
encoding="utf-8",
31+
)
32+
33+
file_b.write_text(
34+
"""= File B
35+
36+
Some content in B.
37+
38+
include::file_a.adoc[]
39+
""",
40+
encoding="utf-8",
41+
)
42+
43+
return tmp_path
44+
45+
46+
@pytest.fixture
47+
def temp_self_circular_include(tmp_path: Path) -> Path:
48+
"""Create a temporary directory with a file that includes itself."""
49+
file_a = tmp_path / "self_ref.adoc"
50+
file_a.write_text(
51+
"""= Self Referencing Doc
52+
53+
include::self_ref.adoc[]
54+
""",
55+
encoding="utf-8",
56+
)
57+
return tmp_path
58+
59+
60+
class TestCircularIncludeValidation:
61+
"""Test that circular includes are reported as errors, not orphaned files."""
62+
63+
def test_circular_include_reported_as_error(
64+
self, temp_circular_include: Path
65+
):
66+
"""Issue #251: Circular includes should be reported as circular_include errors."""
67+
mcp = create_mcp_server(temp_circular_include)
68+
69+
# Access validate_structure tool
70+
tools = {t.name: t for t in mcp._tool_manager._tools.values()}
71+
result = tools["validate_structure"].fn()
72+
73+
# Should NOT be valid due to circular include
74+
assert result["valid"] is False, (
75+
f"Expected valid=False for circular include. Result: {result}"
76+
)
77+
78+
# Should have a circular_include error
79+
error_types = [e["type"] for e in result["errors"]]
80+
assert "circular_include" in error_types, (
81+
f"Expected 'circular_include' error. Errors: {result['errors']}"
82+
)
83+
84+
def test_circular_include_not_reported_as_orphaned(
85+
self, temp_circular_include: Path
86+
):
87+
"""Issue #251: Files involved in circular includes should NOT be reported as orphaned."""
88+
mcp = create_mcp_server(temp_circular_include)
89+
90+
tools = {t.name: t for t in mcp._tool_manager._tools.values()}
91+
result = tools["validate_structure"].fn()
92+
93+
# Check that no orphaned_file warnings exist for the circular files
94+
orphaned_warnings = [
95+
w for w in result["warnings"] if w["type"] == "orphaned_file"
96+
]
97+
orphaned_paths = [w["path"] for w in orphaned_warnings]
98+
99+
assert "file_a.adoc" not in orphaned_paths, (
100+
f"file_a.adoc should not be orphaned. Warnings: {result['warnings']}"
101+
)
102+
assert "file_b.adoc" not in orphaned_paths, (
103+
f"file_b.adoc should not be orphaned. Warnings: {result['warnings']}"
104+
)
105+
106+
def test_circular_include_error_contains_chain(
107+
self, temp_circular_include: Path
108+
):
109+
"""Circular include error should include the include chain."""
110+
mcp = create_mcp_server(temp_circular_include)
111+
112+
tools = {t.name: t for t in mcp._tool_manager._tools.values()}
113+
result = tools["validate_structure"].fn()
114+
115+
circular_errors = [
116+
e for e in result["errors"] if e["type"] == "circular_include"
117+
]
118+
assert len(circular_errors) >= 1
119+
120+
# The error message should mention the files involved
121+
error = circular_errors[0]
122+
assert "message" in error
123+
assert "circular" in error["message"].lower() or "Circular" in error["message"]
124+
125+
def test_self_circular_include_reported_as_error(
126+
self, temp_self_circular_include: Path
127+
):
128+
"""A file that includes itself should be reported as circular_include error."""
129+
mcp = create_mcp_server(temp_self_circular_include)
130+
131+
tools = {t.name: t for t in mcp._tool_manager._tools.values()}
132+
result = tools["validate_structure"].fn()
133+
134+
# Should NOT be valid
135+
assert result["valid"] is False
136+
137+
# Should have a circular_include error
138+
error_types = [e["type"] for e in result["errors"]]
139+
assert "circular_include" in error_types, (
140+
f"Expected 'circular_include' error for self-include. Errors: {result['errors']}"
141+
)
142+
143+
# Should NOT have orphaned_file warning for the file
144+
orphaned_paths = [
145+
w["path"] for w in result["warnings"] if w["type"] == "orphaned_file"
146+
]
147+
assert "self_ref.adoc" not in orphaned_paths
148+
149+
150+
class TestCLICircularIncludeValidation:
151+
"""Test CLI validate command with circular includes."""
152+
153+
def test_cli_validate_reports_circular_include(
154+
self, temp_circular_include: Path
155+
):
156+
"""CLI validate should report circular includes as errors."""
157+
runner = CliRunner()
158+
result = runner.invoke(
159+
cli,
160+
["--docs-root", str(temp_circular_include), "validate"],
161+
)
162+
163+
assert "circular_include" in result.output
164+
# Exit code 4 = EXIT_VALIDATION_ERROR (validation found errors)
165+
assert result.exit_code == 4

0 commit comments

Comments
 (0)