Skip to content

Commit 03ab29a

Browse files
committed
Address review: relative prefix, root source-dir, lint fix
- Remove includedDir != "." guard so root-level includes get source-dir injection (review comment #1) - Compute display prefix relative to the catalog-owning file's directory via filepath.Rel, so links are correct when the includer is in a subdirectory (review comments #2, #3) - Handle source-dir: "." by using RootFS directly - Remove unused resolveGlobMatches wrapper (lint fix) - Add tests for root-include and sibling-subdir edge cases - Update copilot-instructions.md (mdsmith fix) https://claude.ai/code/session_01VN3XGWEbs4qRPet8qBia59
1 parent 46ed7a0 commit 03ab29a

5 files changed

Lines changed: 94 additions & 11 deletions

File tree

.github/copilot-instructions.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ mdsmith — a Markdown linter written in Go.
1616
- [Plan template; see PLAN.md for status, plans live in plan/](../plan/proto.md)
1717

1818
<?catalog
19+
source-dir: "."
1920
glob:
2021
- "docs/**/*.md"
2122
- "!docs/research/**"

internal/rules/catalog/rule.go

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -251,17 +251,40 @@ func splitIncludeExclude(glob string) (include, exclude []string) {
251251
// resolveGlobFS returns the filesystem to use for glob resolution and
252252
// a path prefix for display filenames. When source-dir is set (injected
253253
// by include expansion), globs resolve from that subdirectory of RootFS
254-
// and matched filenames are prefixed so links work from the including file.
254+
// and matched filenames are prefixed relative to the catalog-owning
255+
// file's directory so links work correctly.
255256
func resolveGlobFS(f *lint.File, params map[string]string) (globFS fs.FS, prefix string) {
256257
sourceDir := params["source-dir"]
257258
if sourceDir == "" || f.RootFS == nil {
258259
return f.FS, ""
259260
}
261+
262+
sourceDir = path.Clean(sourceDir)
263+
fileDir := path.Clean(filepath.ToSlash(filepath.Dir(f.Path)))
264+
265+
// Compute prefix relative to the file's directory so display
266+
// filenames produce correct links from the including file.
267+
relPrefix, err := filepath.Rel(fileDir, sourceDir)
268+
if err != nil {
269+
return f.FS, ""
270+
}
271+
relPrefix = filepath.ToSlash(relPrefix)
272+
273+
if sourceDir == "." {
274+
if relPrefix == "." {
275+
return f.RootFS, ""
276+
}
277+
return f.RootFS, relPrefix
278+
}
279+
260280
sub, err := fs.Sub(f.RootFS, sourceDir)
261281
if err != nil {
262282
return f.FS, ""
263283
}
264-
return sub, sourceDir
284+
if relPrefix == "." {
285+
return sub, ""
286+
}
287+
return sub, relPrefix
265288
}
266289

267290
// buildCatalogEntries resolves glob matches, reads front matter, and
@@ -293,14 +316,9 @@ func buildCatalogEntries(f *lint.File, params map[string]string) []fileEntry {
293316
return entries
294317
}
295318

296-
// resolveGlobMatches expands include patterns, filters out exclude and
297-
// gitignore matches, and returns deduplicated file paths.
298-
func resolveGlobMatches(f *lint.File, params map[string]string) []string {
299-
return resolveGlobMatchesFrom(f.FS, f, params)
300-
}
301-
302-
// resolveGlobMatchesFrom is like resolveGlobMatches but uses the given
303-
// FS for glob resolution instead of f.FS.
319+
// resolveGlobMatchesFrom expands include patterns using the given FS,
320+
// filters out exclude and gitignore matches, and returns deduplicated
321+
// file paths.
304322
func resolveGlobMatchesFrom(globFS fs.FS, f *lint.File, params map[string]string) []string {
305323
includePatterns, excludePatterns := splitIncludeExclude(params["glob"])
306324
matcher, base := resolveGitignore(f, params)

internal/rules/catalog/rule_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3257,3 +3257,51 @@ source-dir: "docs"
32573257
// The excluded file should not appear as a link in the generated content.
32583258
assert.NotContains(t, result, "[internal.md](docs/internal.md)")
32593259
}
3260+
3261+
func TestCatalog_SourceDirRoot(t *testing.T) {
3262+
// source-dir: "." means globs resolve from the project root.
3263+
// This happens when a subdirectory file includes a root-level file.
3264+
src := `<?catalog
3265+
glob: "*.md"
3266+
source-dir: "."
3267+
?>
3268+
<?/catalog?>
3269+
`
3270+
mapFS := fstest.MapFS{
3271+
"api.md": {Data: []byte("# API\n")},
3272+
"guide.md": {Data: []byte("# Guide\n")},
3273+
"docs/other.md": {Data: []byte("# Other\n")},
3274+
}
3275+
f := newTestFile(t, "docs/index.md", src, mapFS)
3276+
f.RootFS = mapFS
3277+
r := &Rule{}
3278+
result := string(r.Fix(f))
3279+
3280+
// From docs/index.md, root files need a ../ prefix.
3281+
assert.Contains(t, result, "../api.md")
3282+
assert.Contains(t, result, "../guide.md")
3283+
assert.NotContains(t, result, "other.md")
3284+
}
3285+
3286+
func TestCatalog_SourceDirFromSubdirIncluder(t *testing.T) {
3287+
// When the catalog-owning file is in a subdirectory and source-dir
3288+
// points to a sibling subdirectory, the prefix should be relative.
3289+
src := `<?catalog
3290+
glob: "*.md"
3291+
source-dir: "docs/dev"
3292+
?>
3293+
<?/catalog?>
3294+
`
3295+
mapFS := fstest.MapFS{
3296+
"docs/dev/api.md": {Data: []byte("# API\n")},
3297+
"README.md": {Data: []byte("# Root\n")},
3298+
}
3299+
f := newTestFile(t, "docs/intro.md", src, mapFS)
3300+
f.RootFS = mapFS
3301+
r := &Rule{}
3302+
result := string(r.Fix(f))
3303+
3304+
// From docs/intro.md, docs/dev/api.md is at dev/api.md.
3305+
assert.Contains(t, result, "dev/api.md")
3306+
assert.NotContains(t, result, "docs/dev/api.md")
3307+
}

internal/rules/include/rule.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ func processIncludedContent(
242242
if _, wrapped := params["wrap"]; !wrapped {
243243
includedDir := path.Dir(includedPath)
244244
includerDir := path.Dir(filePath)
245-
if includedDir != includerDir && includedDir != "." {
245+
if includedDir != includerDir {
246246
text = injectSourceDir(text, includedDir)
247247
}
248248
}

internal/rules/include/rule_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,22 @@ func TestFix_NoSourceDirWhenSameDir(t *testing.T) {
611611
assert.NotContains(t, result, "source-dir:")
612612
}
613613

614+
func TestFix_InjectsSourceDirForRootInclude(t *testing.T) {
615+
// When a subdir file includes a root-level file, source-dir: "."
616+
// should be injected so catalog globs resolve from the project root.
617+
fsys := fstest.MapFS{
618+
"root-catalog.md": {Data: []byte(
619+
"<?catalog\nglob: \"*.md\"\n?>\n<?/catalog?>\n",
620+
)},
621+
}
622+
src := "<?include\nfile: ../root-catalog.md\n?>\nold\n<?/include?>\n"
623+
f := newTestFile(t, "docs/index.md", src, fsys)
624+
r := &Rule{}
625+
result := string(r.Fix(f))
626+
627+
assert.Contains(t, result, `source-dir: "."`)
628+
}
629+
614630
// =====================================================================
615631
// No FS
616632
// =====================================================================

0 commit comments

Comments
 (0)