Plan 200: move docs/ embed out of internal/lsp/hover.go#375
Merged
Conversation
internal/lsp/hover.go imported docs/guides/directives as a Go package, putting compiled source inside the docs tree and adding a docs/ layer to the import graph that the architecture map does not have. Move the embed to internal/directives, mirroring the internal/concepts pattern: short hover-sized stubs in internal/directives/*.md hold the per-directive copy that the LSP serves; the full user guides stay in docs/guides/directives/ and remain indexed by the catalog. Adds internal/directives/** to directory-structure.allowed so the new package's markdown lints under default rules.
The mdsmith-fixed-version CI job runs the pinned v0.15.0 binary against .mdsmith.pinned.yml. The previous commit added internal/directives/** to .mdsmith.yml but not to the pinned mirror, so v0.15.0 fired MDS033 on every new markdown file in the package.
Copilot review on PR #374: a copy/pasteable plan catalog example with `sort: id` collates lexicographically (100 before 52). Switch the stub to `sort: numeric:id` so it matches the recommendation in the full generating-content guide.
Contributor
There was a problem hiding this comment.
Pull request overview
Moves LSP hover directive documentation out of the docs/ tree and into a Go-internal package, avoiding compiling Go source under documentation paths and aligning with the repo’s architecture layering.
Changes:
- Replace
internal/lsp/hover.go’s dependency ondocs/guides/directiveswithinternal/directivesembedded stubs. - Add
internal/directives/package with embedded hover-sized Markdown stubs + a unit test to ensure embedded files exist. - Update directory-structure allowlists and mark Plan 200 complete (refresh PLAN catalog entry).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| plan/200_arch-fix-hover-embed.md | Marks Plan 200 complete and updates tasks/acceptance criteria text. |
| PLAN.md | Updates Plan 200 status in the plan catalog. |
| internal/lsp/hover.go | Switches directive hover docs FS import from docs/... to internal/directives. |
| internal/directives/directives.go | New embedded FS for hover stubs (//go:embed *.md). |
| internal/directives/directives_test.go | Adds a unit test to ensure embedded stub files exist and are non-empty. |
| internal/directives/build.md | Adds hover stub content for <?build?>. |
| internal/directives/enforcing-structure.md | Adds hover stub content for <?require?> and <?allow-empty-section?>. |
| internal/directives/generating-content.md | Adds hover stub content for <?catalog?> and <?include?>. |
| docs/guides/directives/embed.go | Removes the Go embed package from docs/ (docs remain as Markdown only). |
| .mdsmith.yml | Allows internal/directives/** under directory-structure.allowed. |
| .mdsmith.pinned.yml | Keeps pinned config in sync with the new allowed directory. |
Two unresolved Copilot threads on PR #375: generating-content.md: the heading-level paragraph was backwards. The include rule only accepts heading-level: "absolute", which SHIFTS included headings to nest under the include site's parent heading. Omitting heading-level keeps source levels. There is no "relative" option — internal/rules/include rejects any value other than "absolute". Rewrite the sentence to match. directives_test.go + new internal/lsp/hover_test.go: the directives-side test only checks a hardcoded list, so the comment claiming it guards directiveToDocFile was misleading. Soften that comment and add the real cross-package invariant test next to the map: walk directiveToDocFile in internal/lsp and assert each mapped file resolves in directives.FS.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Two further Copilot threads on #375: build.md said "No external tool runs at lint time", which implies fix might execute the recipe. mdsmith never runs recipe commands at all — fix only renders the body-template, and MDS040 is a static check. generating-content.md said the include body "is regenerated whenever the source changes", which implies automatic updates. Make it explicit: mdsmith fix regenerates; mdsmith check reports drift.
Comment on lines
+3
to
+23
| import ( | ||
| "io/fs" | ||
| "testing" | ||
|
|
||
| "github.com/jeduden/mdsmith/internal/directives" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| // TestDirectiveToDocFileResolves walks directiveToDocFile and asserts | ||
| // every mapped filename resolves to a non-empty file in directives.FS. | ||
| // Adding a directive→file mapping without shipping the stub would | ||
| // otherwise return an empty payload at hover time. | ||
| func TestDirectiveToDocFileResolves(t *testing.T) { | ||
| for name, fname := range directiveToDocFile { | ||
| t.Run(name, func(t *testing.T) { | ||
| data, err := fs.ReadFile(directives.FS, fname) | ||
| assert.NoError(t, err, "directive %q maps to %q but stub is missing", name, fname) | ||
| assert.NotEmpty(t, data, "directive %q stub %q must have content", name, fname) | ||
| }) | ||
| } | ||
| } |
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
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
Picks up the work from #374 (Plan 200) onto a fresh branch off
current
main, since #374's branch had drifted out of sync.internal/lsp/hover.goimporteddocs/guides/directivesas a Gopackage, which put compiled source inside the docs tree and added a
docs/layer the architecture map doesn't have.internal/directives/holds short, hover-sized stubs (theinternal/concepts/placeholder-grammar.mdpattern) embedded viaembed.FS. Hover now importsinternal/directivesinstead.docs/guides/directives/and remainindexed by
docs/guides/index.md. Onlyembed.gowas deleted there..mdsmith.ymland.mdsmith.pinned.ymlgaininternal/directives/**under
directory-structure.allowedso the new package's markdownlints under default rules.
sort: numeric:idin the catalog stubexample.
Closes the hover entry from
docs/development/architecture-audit.md
("internal/lsp/hover.go imports from docs/").
Test plan
go build ./...go test ./...go tool golangci-lint run— 0 issuesgo run ./cmd/mdsmith check .— 353 files, 0 failuresgrep -rn 'docs/guides/directives' internal/ --include='*.go'returns only the package comment in
internal/directives/directives.goGenerated by Claude Code