feat: add grouped sidebar navigation and command cheat sheet - #25
Conversation
Parse documentation.md TOC into grouped sections with anchors and dynamic section names. Render sidebar groups dynamically instead of hardcoded wrapper. Map README links to root path and strip query params/anchors from paths. Use explicit RedirectResponse constructor in DocsViewer.
Discover commands from deployer-php package source, group by namespace, and link to relevant documentation sections. Adds route, view, service, and navigation header button.
Commands can now reference multiple documentation sections instead of just one. Links display as numbered badges next to the command name.
📝 WalkthroughWalkthroughAdds a Command Cheat Sheet feature (service, route, view, cache), refactors TOC parsing and navigation rendering, updates tests, introduces a docs cheat sheet cache config and IDE metadata, and makes small fixes in DocsViewer redirect handling and a User attribute. Changes
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/Pest.php`:
- Around line 58-81: The ensureDocsPathConfigured function currently assumes a
base .env exists; update it to also resolve DOCS_PATH from CI-friendly sources:
first check config('docs.path') as now, then check getenv('DOCS_PATH') /
$_ENV['DOCS_PATH'] / env('DOCS_PATH'), then attempt to read
base_path('.env.testing') if base_path('.env') is missing or not readable, and
only then throw the RuntimeException. Modify the resolution logic around
file_get_contents and preg_match to try .env.testing when .env is unavailable
and prefer explicit env vars before parsing files so CI variables (phpunit.xml
or exported env) are honored.
🧹 Nitpick comments (5)
tests/Unit/TocParserServiceTest.php (1)
8-29: Test relies on actual documentation content, making it fragile to doc changes.This test validates against specific documentation content ("Guides", "References", exact link counts), which will break if the documentation structure changes. Consider extracting
parseContent()to a testable method or using a test fixture with known content for more isolated unit testing.That said, this approach does provide value as an integration-style test validating real parsing behavior.
💡 Alternative: Test with controlled input
it('parses grouped toc sections from markdown content', function (): void { $content = <<<'MD' <a name="guides"></a> ## Guides - [Introduction](README.md) - [Installation](installation.md) <a name="references"></a> ## References - [Managing Sites](managing-sites.md) MD; // Either mock DocsPathService or expose parseContent for testing $service = new TocParserService(/* mock */); // ... assertions on controlled content });app/Services/CommandCheatSheetService.php (1)
146-178: Regex only matches single-quoted attribute values, but no double-quoted attributes exist in the codebase.The patterns
name:\s*'([^']+)'anddescription:\s*'([^']+)'correctly match only single-quoted strings. However, a search of the entire codebase reveals that no#[AsCommand]attributes currently use double quotes, making this a theoretical concern rather than a practical issue.If defensive support for both quote styles is desired, note that the proposed regex pattern
/name:\s*[\'"]([^\'"]+)[\'"]/' has a flaw: the character class[^\'"]+will fail to match values containing apostrophes (e.g.,"Can't"). A more robust approach would use alternation:'([^']+)'|"([^"]+)"` to properly handle both single and double-quoted strings independently.routes/web.php (1)
14-18: Route implementation is correct.The route properly injects the service and renders the view. The implementation is clean and follows Laravel conventions.
Optional consideration: The
build()method reads and parses files from disk on every request. If this page sees significant traffic, consider caching the result (e.g., usingCache::remember()with a TTL or cache invalidation on deploy).resources/views/cheat-sheet.blade.php (2)
14-39: Consider extracting duplicated header/footer to shared components.The header (lines 15-39) and footer (lines 102-132) appear to duplicate the docs layout. If these sections need updates in the future, they'll need to be changed in multiple places.
Consider extracting these to shared Blade components (e.g.,
<x-header />and<x-footer />) or using a shared layout with a slot for the main content.Also applies to: 101-132
85-92: Add separators between multiple aliases for readability.When a command has multiple aliases, they render adjacent to each other without visual separation. Consider adding commas or other separators.
💅 Proposed fix to add comma separators
`@if` ($command['aliases'] !== []) <span class="ml-1 text-xs text-zinc-400 dark:text-zinc-500"> alias: - `@foreach` ($command['aliases'] as $alias) - <code class="font-mono text-fuchsia-600 dark:text-fuchsia-400">{{ $alias }}</code> + `@foreach` ($command['aliases'] as $index => $alias) + <code class="font-mono text-fuchsia-600 dark:text-fuchsia-400">{{ $alias }}</code>@if ($index < count($command['aliases']) - 1),`@endif` `@endforeach` </span> `@endif`
| function ensureDocsPathConfigured(): string | ||
| { | ||
| $configuredPath = config('docs.path'); | ||
|
|
||
| if (is_string($configuredPath) && $configuredPath !== '') { | ||
| return $configuredPath; | ||
| } | ||
|
|
||
| $envContent = file_get_contents(base_path('.env')); | ||
|
|
||
| if ($envContent === false) { | ||
| throw new RuntimeException('Unable to read .env file to resolve DOCS_PATH.'); | ||
| } | ||
|
|
||
| if (preg_match("/^DOCS_PATH=(?:\"([^\"]+)\"|'([^']+)'|([^\r\n#]+))/m", $envContent, $matches) !== 1) { | ||
| throw new RuntimeException('DOCS_PATH is not configured in tests or .env.'); | ||
| } | ||
|
|
||
| $docsPath = trim($matches[1] ?: $matches[2] ?: $matches[3]); | ||
|
|
||
| config(['docs.path' => $docsPath]); | ||
|
|
||
| return $docsPath; | ||
| } |
There was a problem hiding this comment.
CI pipeline failure: .env file not available in CI environment.
The pipeline is failing because CI environments typically don't have a .env file — they use .env.testing, environment variables, or phpunit.xml configuration instead.
Consider checking multiple sources for the docs path configuration:
🐛 Proposed fix to support CI environments
function ensureDocsPathConfigured(): string
{
$configuredPath = config('docs.path');
if (is_string($configuredPath) && $configuredPath !== '') {
return $configuredPath;
}
- $envContent = file_get_contents(base_path('.env'));
+ // Try .env.testing first (used in CI), then .env
+ $envFile = file_exists(base_path('.env.testing'))
+ ? base_path('.env.testing')
+ : base_path('.env');
+
+ $envContent = file_get_contents($envFile);
if ($envContent === false) {
- throw new RuntimeException('Unable to read .env file to resolve DOCS_PATH.');
+ throw new RuntimeException(
+ 'Unable to read .env file to resolve DOCS_PATH. '
+ . 'Ensure DOCS_PATH is set in config, environment, or .env/.env.testing.'
+ );
}
if (preg_match("/^DOCS_PATH=(?:\"([^\"]+)\"|'([^']+)'|([^\r\n#]+))/m", $envContent, $matches) !== 1) {
throw new RuntimeException('DOCS_PATH is not configured in tests or .env.');
}
$docsPath = trim($matches[1] ?: $matches[2] ?: $matches[3]);
config(['docs.path' => $docsPath]);
return $docsPath;
}Alternatively, ensure DOCS_PATH is defined in phpunit.xml under <php><env> or in .env.testing for CI runs.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function ensureDocsPathConfigured(): string | |
| { | |
| $configuredPath = config('docs.path'); | |
| if (is_string($configuredPath) && $configuredPath !== '') { | |
| return $configuredPath; | |
| } | |
| $envContent = file_get_contents(base_path('.env')); | |
| if ($envContent === false) { | |
| throw new RuntimeException('Unable to read .env file to resolve DOCS_PATH.'); | |
| } | |
| if (preg_match("/^DOCS_PATH=(?:\"([^\"]+)\"|'([^']+)'|([^\r\n#]+))/m", $envContent, $matches) !== 1) { | |
| throw new RuntimeException('DOCS_PATH is not configured in tests or .env.'); | |
| } | |
| $docsPath = trim($matches[1] ?: $matches[2] ?: $matches[3]); | |
| config(['docs.path' => $docsPath]); | |
| return $docsPath; | |
| } | |
| function ensureDocsPathConfigured(): string | |
| { | |
| $configuredPath = config('docs.path'); | |
| if (is_string($configuredPath) && $configuredPath !== '') { | |
| return $configuredPath; | |
| } | |
| // Try .env.testing first (used in CI), then .env | |
| $envFile = file_exists(base_path('.env.testing')) | |
| ? base_path('.env.testing') | |
| : base_path('.env'); | |
| $envContent = file_get_contents($envFile); | |
| if ($envContent === false) { | |
| throw new RuntimeException( | |
| 'Unable to read .env file to resolve DOCS_PATH. ' | |
| . 'Ensure DOCS_PATH is set in config, environment, or .env/.env.testing.' | |
| ); | |
| } | |
| if (preg_match("/^DOCS_PATH=(?:\"([^\"]+)\"|'([^']+)'|([^\r\n#]+))/m", $envContent, $matches) !== 1) { | |
| throw new RuntimeException('DOCS_PATH is not configured in tests or .env.'); | |
| } | |
| $docsPath = trim($matches[1] ?: $matches[2] ?: $matches[3]); | |
| config(['docs.path' => $docsPath]); | |
| return $docsPath; | |
| } |
🧰 Tools
🪛 GitHub Actions: Pest
[error] 66-66: Unable to read .env file to resolve DOCS_PATH. This caused multiple tests to fail during vendor/bin/pest --parallel --coverage --coverage-text execution.
🤖 Prompt for AI Agents
In `@tests/Pest.php` around lines 58 - 81, The ensureDocsPathConfigured function
currently assumes a base .env exists; update it to also resolve DOCS_PATH from
CI-friendly sources: first check config('docs.path') as now, then check
getenv('DOCS_PATH') / $_ENV['DOCS_PATH'] / env('DOCS_PATH'), then attempt to
read base_path('.env.testing') if base_path('.env') is missing or not readable,
and only then throw the RuntimeException. Modify the resolution logic around
file_get_contents and preg_match to try .env.testing when .env is unavailable
and prefer explicit env vars before parsing files so CI variables (phpunit.xml
or exported env) are honored.
Provide a sensible default for DOCS_PATH in config, pointing to the vendor package directory. This eliminates the need for the ensureDocsPathConfigured() test helper that was reading .env directly.
Support both single and double-quoted attribute values in command metadata parsing. Cache the cheat sheet response with a configurable TTL. Add comma separators between command aliases in the view.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@resources/views/cheat-sheet.blade.php`:
- Around line 79-82: When generating link URLs for command links, avoid always
using route('docs.show') because an empty $link['page'] (README/root) leads to
/docs/ and a redirect that can drop anchors; update the rendering logic in the
blade loop that iterates $command['links'] to detect when $link['page'] is empty
and use route('home') (or the homepage route) instead of route('docs.show'),
preserving the #{{ $link['anchor'] }} anchor and keeping the same link
attributes (title, classes, index label) so anchors and UX are not lost by the
extra redirect.
In `@tests/Feature/DocsSidebarNavigationTest.php`:
- Around line 35-42: The test "redirects missing docs pages to the docs home" is
asserting a redirect for '/docs/introduction' but that slug exists and returns
200; update the test in DocsSidebarNavigationTest (the it(...) block) to request
a guaranteed-missing docs slug (e.g., '/docs/non-existent-slug') or otherwise
use a fixture/slug known to be absent, then assertRedirect('/') and
assertStatus(301) against that missing-slug request so the test reflects actual
missing-page behavior.
- Around line 9-32: The test relies on real documentation which can change; make
it deterministic by pinning docs.path to a local fixture before the request:
register or bind a known fixture docs folder (e.g., tests/Fixtures/docs with
documentation.md matching the expected TOC) and set config('docs.path') or use
DocsPathService to point at that fixture in the test; ensure TocParserService
and DocumentService use that path by calling
app()->forgetInstance(DocsPathService::class),
app()->instance(DocsPathService::class, new DocsPathService(/*fixture path*/))
(or set config('docs.path', base_path('tests/Fixtures/docs'))), then proceed
with the $this->get('/') and assertions so the TOC order is stable.
| @if ($command['links'] !== []) | ||
| @foreach ($command['links'] as $index => $link) | ||
| <a href="{{ route('docs.show', ['page' => $link['page']]) }}#{{ $link['anchor'] }}" class="inline-flex items-center justify-center size-4 rounded-sm bg-accent/10 text-[10px] font-medium leading-none text-accent no-underline hover:bg-accent/20 dark:bg-blue-400/10 dark:text-blue-400 dark:hover:bg-blue-400/20" title="{{ $link['page'] }}#{{ $link['anchor'] }}">{{ $index + 1 }}</a> | ||
| @endforeach |
There was a problem hiding this comment.
Handle README/root links without forcing a /docs/ redirect.
If $link['page'] is empty (README → root), generating route('docs.show') yields /docs/ and a redirect; anchors can be lost and this adds extra hops. Consider routing directly to route('home') when page is empty.
🔧 Suggested adjustment
- `@foreach` ($command['links'] as $index => $link)
- <a href="{{ route('docs.show', ['page' => $link['page']]) }}#{{ $link['anchor'] }}" class="inline-flex items-center justify-center size-4 rounded-sm bg-accent/10 text-[10px] font-medium leading-none text-accent no-underline hover:bg-accent/20 dark:bg-blue-400/10 dark:text-blue-400 dark:hover:bg-blue-400/20" title="{{ $link['page'] }}#{{ $link['anchor'] }}">{{ $index + 1 }}</a>
- `@endforeach`
+ `@foreach` ($command['links'] as $index => $link)
+ `@php`
+ $docHref = $link['page'] === ''
+ ? route('home')
+ : route('docs.show', ['page' => $link['page']]);
+ `@endphp`
+ <a href="{{ $docHref }}#{{ $link['anchor'] }}" class="inline-flex items-center justify-center size-4 rounded-sm bg-accent/10 text-[10px] font-medium leading-none text-accent no-underline hover:bg-accent/20 dark:bg-blue-400/10 dark:text-blue-400 dark:hover:bg-blue-400/20" title="{{ $link['page'] }}#{{ $link['anchor'] }}">{{ $index + 1 }}</a>
+ `@endforeach`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @if ($command['links'] !== []) | |
| @foreach ($command['links'] as $index => $link) | |
| <a href="{{ route('docs.show', ['page' => $link['page']]) }}#{{ $link['anchor'] }}" class="inline-flex items-center justify-center size-4 rounded-sm bg-accent/10 text-[10px] font-medium leading-none text-accent no-underline hover:bg-accent/20 dark:bg-blue-400/10 dark:text-blue-400 dark:hover:bg-blue-400/20" title="{{ $link['page'] }}#{{ $link['anchor'] }}">{{ $index + 1 }}</a> | |
| @endforeach | |
| `@if` ($command['links'] !== []) | |
| `@foreach` ($command['links'] as $index => $link) | |
| `@php` | |
| $docHref = $link['page'] === '' | |
| ? route('home') | |
| : route('docs.show', ['page' => $link['page']]); | |
| `@endphp` | |
| <a href="{{ $docHref }}#{{ $link['anchor'] }}" class="inline-flex items-center justify-center size-4 rounded-sm bg-accent/10 text-[10px] font-medium leading-none text-accent no-underline hover:bg-accent/20 dark:bg-blue-400/10 dark:text-blue-400 dark:hover:bg-blue-400/20" title="{{ $link['page'] }}#{{ $link['anchor'] }}">{{ $index + 1 }}</a> | |
| `@endforeach` |
🤖 Prompt for AI Agents
In `@resources/views/cheat-sheet.blade.php` around lines 79 - 82, When generating
link URLs for command links, avoid always using route('docs.show') because an
empty $link['page'] (README/root) leads to /docs/ and a redirect that can drop
anchors; update the rendering logic in the blade loop that iterates
$command['links'] to detect when $link['page'] is empty and use route('home')
(or the homepage route) instead of route('docs.show'), preserving the #{{
$link['anchor'] }} anchor and keeping the same link attributes (title, classes,
index label) so anchors and UX are not lost by the extra redirect.
| it('renders grouped sidebar navigation from documentation toc', function (): void { | ||
| app()->forgetInstance(DocsPathService::class); | ||
| app()->forgetInstance(TocParserService::class); | ||
| app()->forgetInstance(DocumentService::class); | ||
|
|
||
| $response = $this->get('/'); | ||
|
|
||
| $response->assertOk() | ||
| ->assertSeeInOrder([ | ||
| 'Guides', | ||
| 'Introduction', | ||
| 'Installation', | ||
| 'Zero to Deploy', | ||
| 'References', | ||
| 'Managing Sites', | ||
| 'Managing Servers', | ||
| 'Managing Services', | ||
| 'Managing Databases', | ||
| 'Cloud Providers', | ||
| 'Automation & AI', | ||
| ]) | ||
| ->assertSee('href="'.route('home').'"', false) | ||
| ->assertSee('href="'.route('docs.show', ['page' => 'installation']).'"', false) | ||
| ->assertSee('href="'.route('docs.show', ['page' => 'managing-services']).'"', false); |
There was a problem hiding this comment.
Make the TOC order test deterministic.
CI failure indicates the expected order/content doesn’t match the actual docs. Since this test depends on external docs content, it will drift. Consider pinning to a fixture docs set and setting docs.path to that fixture before the request.
✅ Suggested direction
it('renders grouped sidebar navigation from documentation toc', function (): void {
+ config()->set('docs.path', base_path('tests/Fixtures/docs'));
app()->forgetInstance(DocsPathService::class);
app()->forgetInstance(TocParserService::class);
app()->forgetInstance(DocumentService::class);Then add a stable tests/Fixtures/docs/documentation.md that matches the expected order.
🧰 Tools
🪛 GitHub Actions: Pest
[error] 17-17: DocsSidebarNavigationTest: it renders grouped sidebar navigation failed. Expected certain order/content in the rendered HTML.
🤖 Prompt for AI Agents
In `@tests/Feature/DocsSidebarNavigationTest.php` around lines 9 - 32, The test
relies on real documentation which can change; make it deterministic by pinning
docs.path to a local fixture before the request: register or bind a known
fixture docs folder (e.g., tests/Fixtures/docs with documentation.md matching
the expected TOC) and set config('docs.path') or use DocsPathService to point at
that fixture in the test; ensure TocParserService and DocumentService use that
path by calling app()->forgetInstance(DocsPathService::class),
app()->instance(DocsPathService::class, new DocsPathService(/*fixture path*/))
(or set config('docs.path', base_path('tests/Fixtures/docs'))), then proceed
with the $this->get('/') and assertions so the TOC order is stable.
| it('redirects missing docs pages to the docs home', function (): void { | ||
| app()->forgetInstance(DocsPathService::class); | ||
| app()->forgetInstance(TocParserService::class); | ||
| app()->forgetInstance(DocumentService::class); | ||
|
|
||
| $this->get('/docs/introduction') | ||
| ->assertRedirect('/') | ||
| ->assertStatus(301); |
There was a problem hiding this comment.
Redirect expectation is inconsistent with current behavior.
The pipeline shows /docs/introduction returning 200, so this assertion is failing. If you want to test the “missing page” redirect, use a guaranteed-missing slug (or keep a fixture docs set where that slug does not exist).
🔧 Suggested fix
it('redirects missing docs pages to the docs home', function (): void {
+ config()->set('docs.path', base_path('tests/Fixtures/docs'));
app()->forgetInstance(DocsPathService::class);
app()->forgetInstance(TocParserService::class);
app()->forgetInstance(DocumentService::class);
- $this->get('/docs/introduction')
+ $this->get('/docs/definitely-missing-page')
->assertRedirect('/')
->assertStatus(301);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('redirects missing docs pages to the docs home', function (): void { | |
| app()->forgetInstance(DocsPathService::class); | |
| app()->forgetInstance(TocParserService::class); | |
| app()->forgetInstance(DocumentService::class); | |
| $this->get('/docs/introduction') | |
| ->assertRedirect('/') | |
| ->assertStatus(301); | |
| it('redirects missing docs pages to the docs home', function (): void { | |
| config()->set('docs.path', base_path('tests/Fixtures/docs')); | |
| app()->forgetInstance(DocsPathService::class); | |
| app()->forgetInstance(TocParserService::class); | |
| app()->forgetInstance(DocumentService::class); | |
| $this->get('/docs/definitely-missing-page') | |
| ->assertRedirect('/') | |
| ->assertStatus(301); | |
| }); |
🧰 Tools
🪛 GitHub Actions: Pest
[error] 41-41: DocsSidebarNavigationTest: Redirect test failed. Expected redirect to '/' with 301 status, but received 200.
🤖 Prompt for AI Agents
In `@tests/Feature/DocsSidebarNavigationTest.php` around lines 35 - 42, The test
"redirects missing docs pages to the docs home" is asserting a redirect for
'/docs/introduction' but that slug exists and returns 200; update the test in
DocsSidebarNavigationTest (the it(...) block) to request a guaranteed-missing
docs slug (e.g., '/docs/non-existent-slug') or otherwise use a fixture/slug
known to be absent, then assertRedirect('/') and assertStatus(301) against that
missing-slug request so the test reflects actual missing-page behavior.
Summary
Key Changes