feat: refactor cheat sheet to Livewire command index with grouped sections - #30
Merged
Merged
Conversation
Convert the cheat sheet from a closure route to a full Livewire component with cached data. Group commands into semantic sections (Server & Site Operations, Data Services, Cloud Providers, etc.) for better discoverability. Extract shared header and footer into reusable Blade components, replace target="_blank" with wire:navigate on the TOC cheat sheet link, improve code block styling, and add codex to boost config.
Renames the CheatSheet feature to CommandIndex across the entire codebase—Livewire component, service, views, layout, route, config key, env var, sidebar TOC link, and tests.
Replaces the Laravel Cache facade approach with a dedicated file-backed cache service that integrates with optimize:clear. - Add DocsOutputCacheService with SHA1-keyed JSON files, atomic writes via temp-rename, and flock-based double-checked locking - Add DocsClearCommand registered via AppServiceProvider::optimizes() so optimize:clear wipes the docs output cache automatically - Refactor DocsViewer and CommandIndex to cache full page payloads (including TOC) through DocsOutputCacheService - Add DocumentService file fingerprinting (mtime+size) for per-document cache invalidation without TTLs - Replace command_index.cache_ttl_seconds config with unified docs.cache.* config block - Add optimize:clear before optimize in deploy script
Replaces real-docs-dependent tests with isolated fixture-based tests using a shared UsesDocsFixtures concern. Adds full unit coverage for all services (DocumentService, MarkdownService, HeadingExtractorService, CommandIndexService, DocsOutputCacheService, TocParserService) and a Livewire DocsViewer test. Removes superseded feature-level cache tests. Simplifies Pest.php and ArchTest to match actual project structure.
- Remove HomeController (broken redirect to self) and welcome.blade.php - Delete old non-Livewire command-index view - Fix x-slot closing tag in Livewire command-index view - Remove redundant prefix info bar from command-index view - Add PHPDoc array type annotation to DocsOutputCacheService::key()
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughReplaces the static cheat-sheet with a cached, dynamic Command Index and Livewire component; introduces a file-backed DocsOutputCacheService, integrates caching into DocumentService and DocsViewer, registers a docs:clear command, refactors views/layouts into reusable components, and adds fixture-driven tests and test helpers. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
tests/Unit/MarkdownServiceTest.php (1)
93-108: Consider a more stable assertion for alert rendering.Asserting against the specific Tailwind class
dark:border-l-amber-400couples this test to the component's styling implementation. If the alert component's styling is updated, this test will fail even though the behavior is correct.Consider asserting against a more stable marker, such as a semantic class name,
roleattribute, ordata-attribute if available in the component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/MarkdownServiceTest.php` around lines 93 - 108, The test "it('renders important alerts with block content')" couples behavior to a Tailwind class; update the assertion to check for a stable semantic marker instead of 'dark:border-l-amber-400' — for example assert that the generated HTML from app(MarkdownService::class)->toHtml($markdown) contains a role or data attribute like role="alert" or data-alert="important" (or add such an attribute in the alert rendering code if missing) and keep the existing checks for the list items; reference the test block name and the MarkdownService::toHtml call to locate where to change the assertion.tests/Fixtures/docs/docs/link-behavior.md (1)
7-7: Invalid anchor reference may cause test ambiguity.The anchor
#local-anchordoesn't correspond to any heading in this document. If this is intentional to test invalid anchor handling, consider adding a comment to clarify. Otherwise, add a heading like## Local Anchorto make the link valid.Option 1: Add a valid anchor target
[Anchor](`#local-anchor`) [External](https://example.com/docs) + +## Local Anchor + +This section provides the target for the anchor link above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Fixtures/docs/docs/link-behavior.md` at line 7, The link target `#local-anchor` in tests/Fixtures/docs/docs/link-behavior.md points to no heading and creates ambiguity; either add a matching heading (e.g., insert `## Local Anchor` somewhere in the document to create the `#local-anchor` target) or explicitly mark the link as intentionally invalid with a short comment near the link explaining it tests invalid-anchor behavior so readers know it's deliberate.resources/views/components/docs/footer.blade.php (1)
21-23: Consider using Flux icon for consistency.The inline SVG heart icon could be replaced with a Flux icon component for consistency with the other icons in this file (
flux:icon.github,flux:icon.reddit,flux:icon.x). However, this is a minor style preference.Optional: Use flux:icon.heart if available
<span>Made with tender love</span> - <svg class="inline-flex shrink-0 text-zinc-400 [:where(&)]:size-4" data-flux-icon="" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" aria-hidden="true" data-slot="icon"> - <path d="M2 6.342a3.375 3.375 0 0 1 6-2.088 3.375 3.375 0 0 1 5.997 2.26c-.063 2.134-1.618 3.76-2.955 4.784a14.437 14.437 0 0 1-2.676 1.61c-.02.01-.038.017-.05.022l-.014.006-.004.002h-.002a.75.75 0 0 1-.592.001h-.002l-.004-.003-.015-.006a5.528 5.528 0 0 1-.232-.107 14.395 14.395 0 0 1-2.535-1.557C3.564 10.22 1.999 8.558 1.999 6.38L2 6.342Z"></path> - </svg> + <flux:icon.heart class="inline-flex size-4 shrink-0 text-zinc-400" /> <span>and care</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/views/components/docs/footer.blade.php` around lines 21 - 23, Replace the inline SVG heart markup with the project's Flux icon component (e.g., flux:icon.heart or the x-flux component variant) so it matches the other icons like flux:icon.github, flux:icon.reddit and flux:icon.x; preserve the existing attributes and classes (class="inline-flex shrink-0 text-zinc-400 [:where(&)]:size-4", data-flux-icon, view/accessibility attrs such as aria-hidden and data-slot) when instantiating the Flux icon component so visual styling and a11y remain identical.tests/Feature/DocsClearCommandTest.php (1)
8-27: Consider cleaning up the test cache directory after the test.The test creates a unique cache directory under
storage/framework/cache/docs-output-tests/but doesn't clean it up after completion. Over time, repeated test runs could accumulate orphaned directories.♻️ Add afterEach cleanup
afterEach(function (): void { $cachePath = config('docs.cache.path'); if ($cachePath && File::isDirectory($cachePath)) { File::deleteDirectory($cachePath); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Feature/DocsClearCommandTest.php` around lines 8 - 27, The test leaves a per-run cache directory under storage/framework/cache/docs-output-tests/ (created in the it(...) test using DocsOutputCacheService::class and config()->set('docs.cache.path', ...)), so add an afterEach teardown that reads the configured cache path via config('docs.cache.path') and, if it exists and File::isDirectory(...) is true, removes it with File::deleteDirectory(...); place this afterEach alongside the test so the unique test cache is cleaned up after each run.app/Services/DocsOutputCacheService.php (1)
113-140: Legitimatenullvalues cannot be cached and will trigger re-computation on every call.In
remember(), returningnullfrom the resolver bypasses caching (Line 134), and a cachednullwill be treated as a cache miss (Line 121). If any resolver legitimately returnsnull, this creates unnecessary overhead.Consider documenting this behavior or using a sentinel value/wrapper if
nullis a valid cached value.📝 Document the null behavior in the method docblock
+ /** + * Get or compute and cache a value. + * + * Note: Null values are not cached. If the resolver returns null, + * it will be called again on subsequent requests. + */ public function remember(string $key, callable $callback): mixed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Services/DocsOutputCacheService.php` around lines 113 - 140, The remember() method treats null as a cache-miss and thus re-computes resolvers that legitimately return null; update remember (and/or get/put) to support caching explicit nulls by using a sentinel wrapper (e.g., wrap values in a CacheValue object or store a special marker string/array) when storing and unwrapping on retrieval so null is distinguishable from "not present", or alternatively store an additional boolean flag indicating presence; update withLock, get, and put usages in DocsOutputCacheService::remember to wrap/unwrap values atomically, and add a docblock on remember() explaining that nulls are now supported via the sentinel/wrapper and how callers should interpret returned values.app/Services/CommandIndexService.php (1)
198-207: Edge case: commands without a colon return the full command name as namespace.When a command has no colon separator (e.g.,
deploy),explode(':', $command, 2)returns['deploy'], and$segments[0]returns'deploy'instead of'general'. This may be intentional, but consider if standalone commands should be grouped under'general'or their own name.♻️ Optional: group standalone commands under 'general'
private function extractNamespace(string $command): string { $segments = explode(':', $command, 2); - if ($segments[0] === '') { + if ($segments[0] === '' || count($segments) === 1) { return 'general'; } return $segments[0]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Services/CommandIndexService.php` around lines 198 - 207, The extractNamespace method currently returns the whole command when there is no colon (e.g., "deploy"); update extractNamespace(string $command) to treat standalone commands as belonging to the "general" namespace by checking for the presence of a colon (e.g., use strpos($command, ':') === false or count(explode(...)) < 2) and return 'general' in that case, otherwise keep the existing logic that returns the segment before the colon.resources/views/components/docs/toc.blade.php (1)
17-25: Consider removing the external link icon for an internal route.The
square-arrow-out-up-righticon typically indicates navigation to an external site. Since/command-indexis an internal route within the same application, this icon may mislead users into thinking they're leaving the docs site.♻️ Option: Remove the icon or use a different one
<flux:navlist.item href="{{ route('command-index') }}" wire:navigate :current="$currentPath === 'command-index'"> - <div class="flex items-center gap-2"> - <span>Command Index</span> - - <flux:icon.square-arrow-out-up-right class="size-3" /> - </div> + Command Index </flux:navlist.item>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/views/components/docs/toc.blade.php` around lines 17 - 25, The external-link icon (flux:icon.square-arrow-out-up-right) is misleading for the internal route rendered by flux:navlist.item (href="{{ route('command-index') }}"); remove the flux:icon.square-arrow-out-up-right element from the Command Index nav item or replace it with an internal/navigation-appropriate icon (e.g., flux:icon.chevron-right or a simple caret) so the visual affordance matches internal navigation; update the markup inside the flux:navlist.item's div (the element containing <span>Command Index</span>) accordingly.tests/Unit/DocumentServiceTest.php (1)
61-86: Consider extracting fingerprint logic to avoid test brittleness.The test manually replicates the fingerprint computation from
DocumentService::fingerprint(). If the fingerprint format changes, this test will silently break. Consider either makingfingerprint()protected/package-visible for testing, or accepting this coupling as intentional for integration testing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/DocumentServiceTest.php` around lines 61 - 86, The test duplicates the fingerprint computation from DocumentService::fingerprint(), which makes it brittle; change the test to call the service's fingerprint logic instead of reimplementing it (e.g., expose fingerprint() for test usage or add a package-visible/protected accessor on DocumentService and call that in the test) so the cache key uses DocumentService::fingerprint() directly when building the cache key for DocsOutputCacheService::key('document', [...]); update the test to retrieve the fingerprint from the service and use that value when putting the cached payload.app/Livewire/DocsViewer.php (1)
93-110: Consider redirect status code for missing content.The 301 (permanent) redirects for missing README or pages are browser-cacheable, meaning users won't re-check the server for those URLs. If documentation pages might be added later under previously-missing slugs, consider using 302 (temporary) or 307 redirects instead.
📋 If temporary redirects are preferred
if ($readme === null) { - throw new HttpResponseException(new RedirectResponse('/', 301)); + throw new HttpResponseException(new RedirectResponse('/', 302)); }if ($document === null) { - throw new HttpResponseException(new RedirectResponse('/', 301)); + throw new HttpResponseException(new RedirectResponse('/', 302)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Livewire/DocsViewer.php` around lines 93 - 110, The redirects for missing content in DocsViewer currently use RedirectResponse with a 301 status (see the HttpResponseException wrapping RedirectResponse when $readme === null and when $document === null); change those to a non-permanent redirect (e.g., use status 302 or 307) so browsers won’t cache the missing-slug redirect permanently—update both RedirectResponse(...) instantiations in the class/method that handles $readme and $document checks to use the chosen temporary status code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/views/components/docs/header.blade.php`:
- Line 19: The external GitHub link rendered by the flux:button component
currently opens with target="_blank" but lacks rel="noopener"; update the
flux:button invocation (the element with icon="github", size="sm",
href="https://github.com/loadinglucian/deployer-php/") to include rel="noopener"
(or rel="noopener noreferrer") so the new tab cannot access window.opener and to
mitigate tab-nabbing attacks.
---
Nitpick comments:
In `@app/Livewire/DocsViewer.php`:
- Around line 93-110: The redirects for missing content in DocsViewer currently
use RedirectResponse with a 301 status (see the HttpResponseException wrapping
RedirectResponse when $readme === null and when $document === null); change
those to a non-permanent redirect (e.g., use status 302 or 307) so browsers
won’t cache the missing-slug redirect permanently—update both
RedirectResponse(...) instantiations in the class/method that handles $readme
and $document checks to use the chosen temporary status code.
In `@app/Services/CommandIndexService.php`:
- Around line 198-207: The extractNamespace method currently returns the whole
command when there is no colon (e.g., "deploy"); update extractNamespace(string
$command) to treat standalone commands as belonging to the "general" namespace
by checking for the presence of a colon (e.g., use strpos($command, ':') ===
false or count(explode(...)) < 2) and return 'general' in that case, otherwise
keep the existing logic that returns the segment before the colon.
In `@app/Services/DocsOutputCacheService.php`:
- Around line 113-140: The remember() method treats null as a cache-miss and
thus re-computes resolvers that legitimately return null; update remember
(and/or get/put) to support caching explicit nulls by using a sentinel wrapper
(e.g., wrap values in a CacheValue object or store a special marker
string/array) when storing and unwrapping on retrieval so null is
distinguishable from "not present", or alternatively store an additional boolean
flag indicating presence; update withLock, get, and put usages in
DocsOutputCacheService::remember to wrap/unwrap values atomically, and add a
docblock on remember() explaining that nulls are now supported via the
sentinel/wrapper and how callers should interpret returned values.
In `@resources/views/components/docs/footer.blade.php`:
- Around line 21-23: Replace the inline SVG heart markup with the project's Flux
icon component (e.g., flux:icon.heart or the x-flux component variant) so it
matches the other icons like flux:icon.github, flux:icon.reddit and flux:icon.x;
preserve the existing attributes and classes (class="inline-flex shrink-0
text-zinc-400 [:where(&)]:size-4", data-flux-icon, view/accessibility attrs
such as aria-hidden and data-slot) when instantiating the Flux icon component so
visual styling and a11y remain identical.
In `@resources/views/components/docs/toc.blade.php`:
- Around line 17-25: The external-link icon
(flux:icon.square-arrow-out-up-right) is misleading for the internal route
rendered by flux:navlist.item (href="{{ route('command-index') }}"); remove the
flux:icon.square-arrow-out-up-right element from the Command Index nav item or
replace it with an internal/navigation-appropriate icon (e.g.,
flux:icon.chevron-right or a simple caret) so the visual affordance matches
internal navigation; update the markup inside the flux:navlist.item's div (the
element containing <span>Command Index</span>) accordingly.
In `@tests/Feature/DocsClearCommandTest.php`:
- Around line 8-27: The test leaves a per-run cache directory under
storage/framework/cache/docs-output-tests/ (created in the it(...) test using
DocsOutputCacheService::class and config()->set('docs.cache.path', ...)), so add
an afterEach teardown that reads the configured cache path via
config('docs.cache.path') and, if it exists and File::isDirectory(...) is true,
removes it with File::deleteDirectory(...); place this afterEach alongside the
test so the unique test cache is cleaned up after each run.
In `@tests/Fixtures/docs/docs/link-behavior.md`:
- Line 7: The link target `#local-anchor` in
tests/Fixtures/docs/docs/link-behavior.md points to no heading and creates
ambiguity; either add a matching heading (e.g., insert `## Local Anchor`
somewhere in the document to create the `#local-anchor` target) or explicitly
mark the link as intentionally invalid with a short comment near the link
explaining it tests invalid-anchor behavior so readers know it's deliberate.
In `@tests/Unit/DocumentServiceTest.php`:
- Around line 61-86: The test duplicates the fingerprint computation from
DocumentService::fingerprint(), which makes it brittle; change the test to call
the service's fingerprint logic instead of reimplementing it (e.g., expose
fingerprint() for test usage or add a package-visible/protected accessor on
DocumentService and call that in the test) so the cache key uses
DocumentService::fingerprint() directly when building the cache key for
DocsOutputCacheService::key('document', [...]); update the test to retrieve the
fingerprint from the service and use that value when putting the cached payload.
In `@tests/Unit/MarkdownServiceTest.php`:
- Around line 93-108: The test "it('renders important alerts with block
content')" couples behavior to a Tailwind class; update the assertion to check
for a stable semantic marker instead of 'dark:border-l-amber-400' — for example
assert that the generated HTML from
app(MarkdownService::class)->toHtml($markdown) contains a role or data attribute
like role="alert" or data-alert="important" (or add such an attribute in the
alert rendering code if missing) and keep the existing checks for the list
items; reference the test block name and the MarkdownService::toHtml call to
locate where to change the assertion.
|
|
||
| <div class="flex items-center gap-3"> | ||
| {{-- GitHub Button --}} | ||
| <flux:button icon="github" size="sm" href="https://github.com/loadinglucian/deployer-php/" target="_blank">GitHub</flux:button> |
There was a problem hiding this comment.
Add rel="noopener" to external link for security.
The GitHub button opens in a new tab but lacks rel="noopener". This is a security best practice to prevent the new page from accessing window.opener, which could enable tab-nabbing attacks.
🛡️ Proposed fix
- <flux:button icon="github" size="sm" href="https://github.com/loadinglucian/deployer-php/" target="_blank">GitHub</flux:button>
+ <flux:button icon="github" size="sm" href="https://github.com/loadinglucian/deployer-php/" target="_blank" rel="noopener">GitHub</flux:button>📝 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.
Suggested change
| <flux:button icon="github" size="sm" href="https://github.com/loadinglucian/deployer-php/" target="_blank">GitHub</flux:button> | |
| <flux:button icon="github" size="sm" href="https://github.com/loadinglucian/deployer-php/" target="_blank" rel="noopener">GitHub</flux:button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/views/components/docs/header.blade.php` at line 19, The external
GitHub link rendered by the flux:button component currently opens with
target="_blank" but lacks rel="noopener"; update the flux:button invocation (the
element with icon="github", size="sm",
href="https://github.com/loadinglucian/deployer-php/") to include rel="noopener"
(or rel="noopener noreferrer") so the new tab cannot access window.opener and to
mitigate tab-nabbing attacks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CommandIndexcomponent with grouped sections, matching the docs site architectureDocsOutputCacheServicefor caching rendered output to diskHomeController,welcome.blade.php, old non-Livewirecommand-index.blade.phpx-slotclosing tag syntax and removes the redundant prefix info bar from the command index viewTest plan
php artisan test --compact tests/Feature/to confirm all feature tests pass/command-indexand verify commands render in grouped sections