Skip to content

Commit f29a697

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 589d01f commit f29a697

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

internal/rules/include/rule.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ func processIncludedContent(
254254
if _, wrapped := params["wrap"]; !wrapped {
255255
includedDir := path.Dir(includedPath)
256256
includerDir := path.Dir(filePath)
257-
if includedDir != includerDir && includedDir != "." {
257+
if includedDir != includerDir {
258258
text = injectSourceDir(text, includedDir)
259259
}
260260
}

internal/rules/include/rule_test.go

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

736+
func TestFix_InjectsSourceDirForRootInclude(t *testing.T) {
737+
// When a subdir file includes a root-level file, source-dir: "."
738+
// should be injected so catalog globs resolve from the project root.
739+
fsys := fstest.MapFS{
740+
"root-catalog.md": {Data: []byte(
741+
"<?catalog\nglob: \"*.md\"\n?>\n<?/catalog?>\n",
742+
)},
743+
}
744+
src := "<?include\nfile: ../root-catalog.md\n?>\nold\n<?/include?>\n"
745+
f := newTestFile(t, "docs/index.md", src, fsys)
746+
r := &Rule{}
747+
result := string(r.Fix(f))
748+
749+
assert.Contains(t, result, `source-dir: "."`)
750+
}
751+
736752
// =====================================================================
737753
// No FS
738754
// =====================================================================

0 commit comments

Comments
 (0)