-
Notifications
You must be signed in to change notification settings - Fork 25
fix: walk up directory tree to find manifest.json #107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,20 +1,47 @@ | ||||||
| import fs from "node:fs"; | ||||||
| import path from "node:path"; | ||||||
| import { PluginManifest } from "../types/manifest.js"; | ||||||
|
|
||||||
| let cachedManifest: PluginManifest | null | undefined; | ||||||
|
|
||||||
| /** | ||||||
| * Walks up from `startDir` looking for a file named `manifest.json` | ||||||
| * that contains an `id` field (Obsidian plugin manifest). | ||||||
| * Returns the parsed manifest, or null if none is found. | ||||||
| */ | ||||||
| function findManifest(startDir: string): PluginManifest | null { | ||||||
| let dir = startDir; | ||||||
|
|
||||||
| while (true) { | ||||||
| const candidate = path.join(dir, "manifest.json"); | ||||||
|
|
||||||
| try { | ||||||
| const data = fs.readFileSync(candidate, "utf8"); | ||||||
| const parsed = JSON.parse(data); | ||||||
|
|
||||||
| // Obsidian plugin manifests always have an `id` field — | ||||||
| // skip unrelated manifest.json files (e.g. Chrome extensions). | ||||||
| if (parsed && typeof parsed === "object" && typeof parsed.id === "string") { | ||||||
| return parsed as PluginManifest; | ||||||
| } | ||||||
| } catch { | ||||||
| // File doesn't exist or isn't valid JSON — keep walking. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is ignoring too many errors. We should distinguish between when the file doesn't exist and when a manifest file is invalid JSON. Let's maintain the existing behavior when the JSON manifest file we find is invalid. |
||||||
| } | ||||||
|
|
||||||
| const parent = path.dirname(dir); | ||||||
| if (parent === dir) { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should likely not traverse outside of a Git repo either. Check if the directory is a git root and if so stop traversing. |
||||||
| // Reached filesystem root without finding a manifest. | ||||||
| return null; | ||||||
| } | ||||||
| dir = parent; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| export function getManifest(): PluginManifest | null { | ||||||
| if (cachedManifest !== undefined) { | ||||||
| return cachedManifest; | ||||||
| } | ||||||
|
|
||||||
| try { | ||||||
| const data = fs.readFileSync("manifest.json", "utf8"); | ||||||
| cachedManifest = JSON.parse(data); | ||||||
| return cachedManifest as PluginManifest; | ||||||
| } catch (err) { | ||||||
| console.error("Failed to load JSON file:", err); | ||||||
| cachedManifest = null; | ||||||
| return cachedManifest; | ||||||
| } | ||||||
| cachedManifest = findManifest(process.cwd()); | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| return cachedManifest; | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should warn if we find a valid manifest.json that does not include a valid ID. Otherwise that becomes unexpectedly invisible.