Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/docs/Choices/CaptureChoice.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ If you have a tag called `#people`, and you type `#people` in the _Capture To_ f
- **After selection** - Preserves selected text and places the link after it
- **End of line** - Places the link at the end of the current line
- **New line** - Places the link on a new line below the cursor
- **In frontmatter property** - Adds the link to a frontmatter property

## Canvas Capture Notes

Expand Down
1 change: 1 addition & 0 deletions docs/docs/Choices/TemplateChoice.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ When either enabled mode is selected, you can choose where the link is placed:
- **After selection** - Preserves selected text and places the link after it
- **End of line** - Places the link at the end of the current line
- **New line** - Places the link on a new line below the cursor
- **In frontmatter property** - Adds the link to a frontmatter property


**If the target file already exists**. Choose whether QuickAdd should ask what to do, update the existing file, create another file, or keep the existing file.
Expand Down
31 changes: 31 additions & 0 deletions src/gui/ChoiceBuilder/captureChoiceBuilder.ts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 frontmatterProperty silently dropped when toggling link mode between required/optional

When a user toggles the "Enabled (requires active file)" / "Enabled (skip if no active file)" dropdown while the placement is set to "In frontmatter property", the onChange handler constructs a new appendLink object that includes placement, requireActiveFile, and linkType, but omits frontmatterProperty. The normalizedOptions captured at the top of addAppendLinkSetting() contains the frontmatterProperty value (via normalizeAppendLinkOptions at src/types/linkPlacement.ts:85), but it is not forwarded. After this.reload(), the frontmatter property text field resets to empty and the user's configured property name is lost.

The same issue exists in templateChoiceBuilder.ts:336-353.

(Refers to lines 791-806)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type { LinkPlacement, LinkType } from "../../types/linkPlacement";
import {
normalizeAppendLinkOptions,
placementSupportsEmbed,
placementSupportsFrontmatter,
} from "../../types/linkPlacement";
import { getAllFolderPathsInVault } from "../../utilityObsidian";
import { sortFolderPathsByTree } from "../../utils/folder-sorting";
Expand Down Expand Up @@ -819,6 +820,7 @@ export class CaptureChoiceBuilder extends ChoiceBuilder {
dropdown.addOption("afterSelection", "After selection");
dropdown.addOption("endOfLine", "End of line");
dropdown.addOption("newLine", "New line");
dropdown.addOption("inFrontmatter", "In frontmatter property");

dropdown.setValue(normalizedOptions.placement);
dropdown.onChange((value: LinkPlacement) => {
Expand Down Expand Up @@ -875,6 +877,35 @@ export class CaptureChoiceBuilder extends ChoiceBuilder {
});
});
}

if (placementSupportsFrontmatter(normalizedOptions.placement)) {
const linkTypeSetting: Setting = new Setting(this.contentEl);
const current = typeof this.choice.appendLink !== "boolean" ? this.choice.appendLink.frontmatterProperty : '';
linkTypeSetting
.setName("Frontmatter property")
.setDesc("Choose the frontmatter property to insert the link into.")
.addText((text) => {
text.setValue(current ?? '')
text.onChange((value: string) => {
const currentValue = this.choice.appendLink;
const requireActiveFile =
typeof currentValue === "boolean"
? normalizedOptions.requireActiveFile
: currentValue.requireActiveFile;
const placement =
typeof currentValue === "boolean"
? normalizedOptions.placement
: currentValue.placement;

this.choice.appendLink = {
enabled: true,
placement,
requireActiveFile,
frontmatterProperty: value,
};
});
});
}
Comment on lines +881 to +909

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing linkType field when updating appendLink.

When the user changes the frontmatter property (lines 900-905), the code reconstructs this.choice.appendLink but omits the linkType field. This means that if the user had previously selected an embed link type, changing the frontmatter property will silently lose that setting and revert to undefined/default link type.

🔧 Proposed fix
 		if (placementSupportsFrontmatter(normalizedOptions.placement)) {
-			const linkTypeSetting: Setting = new Setting(this.contentEl);
+			const frontmatterPropertySetting: Setting = new Setting(this.contentEl);
 			const current = typeof this.choice.appendLink !== "boolean" ? this.choice.appendLink.frontmatterProperty : '';
-			linkTypeSetting
+			frontmatterPropertySetting
 				.setName("Frontmatter property")
 				.setDesc("Choose the frontmatter property to insert the link into.")
 				.addText((text) => {
 					text.setValue(current ?? '')
 					text.onChange((value: string) => {
 						const currentValue = this.choice.appendLink;
 						const requireActiveFile =
 							typeof currentValue === "boolean"
 								? normalizedOptions.requireActiveFile
 								: currentValue.requireActiveFile;
 						const placement =
 							typeof currentValue === "boolean"
 								? normalizedOptions.placement
 								: currentValue.placement;
+						const linkType =
+							typeof currentValue === "boolean"
+								? normalizedLinkType
+								: currentValue.linkType ?? normalizedLinkType;

 						this.choice.appendLink = {
 							enabled: true,
 							placement,
 							requireActiveFile,
+							linkType,
 							frontmatterProperty: value,
 						};
 					});
 				});
 		}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/gui/ChoiceBuilder/captureChoiceBuilder.ts` around lines 881 - 908, When
rebuilding this.choice.appendLink in the frontmatter property text.onChange
handler, preserve the existing linkType instead of omitting it: read linkType
from currentValue when it's an object (or fall back to
normalizedOptions.linkType/default) and include linkType in the new object along
with enabled, placement, requireActiveFile, and frontmatterProperty so the
user's previously selected link type (e.g., embed) is not lost; modify the
text.onChange block that updates this.choice.appendLink to include linkType
sourced from currentValue || normalizedOptions.

}
}

Expand Down
31 changes: 31 additions & 0 deletions src/gui/ChoiceBuilder/templateChoiceBuilder.ts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 frontmatterProperty silently dropped when toggling link mode in template choice builder

Same issue as in the capture choice builder: when a user toggles between "Enabled (requires active file)" and "Enabled (skip if no active file)" while placement is "In frontmatter property", the frontmatterProperty is not included in the new appendLink object (lines 337-342 and 345-350). The property name is silently lost after this.reload().

(Refers to lines 336-353)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type { LinkPlacement, LinkType } from "../../types/linkPlacement";
import {
normalizeAppendLinkOptions,
placementSupportsEmbed,
placementSupportsFrontmatter,
} from "../../types/linkPlacement";
import { getAllFolderPathsInVault } from "../../utilityObsidian";
import { sortFolderPathsByTree } from "../../utils/folder-sorting";
Expand Down Expand Up @@ -364,6 +365,7 @@ export class TemplateChoiceBuilder extends ChoiceBuilder {
dropdown.addOption("afterSelection", "After selection");
dropdown.addOption("endOfLine", "End of line");
dropdown.addOption("newLine", "New line");
dropdown.addOption("inFrontmatter", "In frontmatter property");

dropdown.setValue(normalizedOptions.placement);
dropdown.onChange((value: LinkPlacement) => {
Expand Down Expand Up @@ -419,6 +421,35 @@ export class TemplateChoiceBuilder extends ChoiceBuilder {
});
});
}

if (placementSupportsFrontmatter(normalizedOptions.placement)) {
const linkTypeSetting: Setting = new Setting(this.contentEl);
const current = typeof this.choice.appendLink !== "boolean" ? this.choice.appendLink.frontmatterProperty : '';
linkTypeSetting
.setName("Frontmatter property")
.setDesc("Choose the frontmatter property to insert the link into.")
.addText((text) => {
text.setValue(current ?? '')
text.onChange((value: string) => {
const currentValue = this.choice.appendLink;
const requireActiveFile =
typeof currentValue === "boolean"
? normalizedOptions.requireActiveFile
: currentValue.requireActiveFile;
const placement =
typeof currentValue === "boolean"
? normalizedOptions.placement
: currentValue.placement;

this.choice.appendLink = {
enabled: true,
placement,
requireActiveFile,
frontmatterProperty: value,
};
});
});
}
}
}

Expand Down
13 changes: 13 additions & 0 deletions src/types/linkPlacement.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
normalizeAppendLinkOptions,
isAppendLinkEnabled,
placementSupportsEmbed,
placementSupportsFrontmatter,
} from "./linkPlacement";

describe("LinkPlacement", () => {
Expand Down Expand Up @@ -126,6 +127,7 @@ describe("LinkPlacement", () => {
"afterSelection",
"endOfLine",
"newLine",
"inFrontmatter"
];

for (const placement of placements) {
Expand All @@ -145,6 +147,17 @@ describe("LinkPlacement", () => {
expect(placementSupportsEmbed("afterSelection")).toBe(false);
expect(placementSupportsEmbed("endOfLine")).toBe(false);
expect(placementSupportsEmbed("newLine")).toBe(false);
expect(placementSupportsEmbed("inFrontmatter")).toBe(false);
});
});

describe("placementSupportsFrontmatter", () => {
it("should return true only for inFrontmatter placement", () => {
expect(placementSupportsFrontmatter("replaceSelection")).toBe(false);
expect(placementSupportsFrontmatter("afterSelection")).toBe(false);
expect(placementSupportsFrontmatter("endOfLine")).toBe(false);
expect(placementSupportsFrontmatter("newLine")).toBe(false);
expect(placementSupportsFrontmatter("inFrontmatter")).toBe(true);
});
});
});
12 changes: 11 additions & 1 deletion src/types/linkPlacement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,19 @@ export type LinkPlacement =
| "replaceSelection" // Replace the current selection with the link
| "afterSelection" // Insert the link after the current selection
| "endOfLine" // Insert the link at the end of the current line
| "newLine"; // Insert the link on a new line
| "newLine" // Insert the link on a new line
| "inFrontmatter"; // Insert the link into a frontmatter field

export type LinkType = "link" | "embed";

export function placementSupportsEmbed(placement: LinkPlacement): boolean {
return placement === "replaceSelection";
}

export function placementSupportsFrontmatter(placement : LinkPlacement) : boolean {
return placement === "inFrontmatter";
}

function sanitizeLinkType(
linkType: LinkType | undefined,
placement: LinkPlacement,
Expand Down Expand Up @@ -41,6 +46,10 @@ export interface AppendLinkOptions {
* Defaults to "link" for legacy settings.
*/
linkType?: LinkType;
/**
* Controls the frontmatter property the link is added to. Only used when placement is inFrontmatter.
*/
frontmatterProperty? : string
}

/**
Expand Down Expand Up @@ -73,6 +82,7 @@ export function normalizeAppendLinkOptions(appendLink: boolean | AppendLinkOptio
placement,
requireActiveFile: appendLink.requireActiveFile ?? true,
linkType: sanitizeLinkType(appendLink.linkType, placement),
frontmatterProperty: appendLink.frontmatterProperty
};
}

Expand Down
33 changes: 30 additions & 3 deletions src/utilityObsidian.ts
Original file line number Diff line number Diff line change
Expand Up @@ -553,9 +553,9 @@ export function insertLinkWithPlacement(
app: App,
text: string,
mode: LinkPlacement = "replaceSelection",
options: { requireActiveView?: boolean; } = {},
options: { requireActiveView?: boolean; frontmatterProperty?: string} = {},
) {
const { requireActiveView = true } = options;
const { requireActiveView = true, frontmatterProperty = '' } = options;
const view = app.workspace.getActiveViewOfType(MarkdownView);
if (!view) {
const message = "Cannot append link because no active Markdown view is available.";
Expand Down Expand Up @@ -584,6 +584,33 @@ export function insertLinkWithPlacement(
editor.replaceSelection(text);
return;
}

//////////////////////////////////////////////////////////////////
// FRONTMATTER-SELECTION
//////////////////////////////////////////////////////////////////

if (mode === "inFrontmatter") {
if(!frontmatterProperty) {
const message = "Invalid frontmatter property: " + frontmatterProperty;
throw new Error(message);
}
const file = view.file;
if(!file) {
const message = "Could not find file of active view";
throw new Error(message);
}
app.fileManager.processFrontMatter(file, (frontmatter) => {
if (frontmatter[frontmatterProperty] === undefined) {
frontmatter[frontmatterProperty] = []
}
if (!Array.isArray(frontmatter[frontmatterProperty])) {
const message = "Could not add into non array property:" + frontmatterProperty;
throw new Error(message)
}
frontmatter[frontmatterProperty].push(text)
})
return
Comment on lines +602 to +612

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 processFrontMatter is async but not awaited, causing silent failures and race conditions

app.fileManager.processFrontMatter() returns a Promise<void> (confirmed by every other callsite in the codebase using await, e.g. src/engine/QuickAddEngine.ts:286), but the call at line 602 is not awaited. This causes three problems:

  1. The function returns (and insertFileLinkToActiveView at src/utilityObsidian.ts:768 returns true) before the frontmatter write has actually completed.
  2. The error thrown inside the callback for non-array properties (line 608) becomes an unhandled promise rejection instead of propagating to the caller.
  3. Subsequent operations after the insertFileLinkToActiveView call (e.g., opening the file in src/engine/TemplateChoiceEngine.ts:144) can race with the pending frontmatter write.

Since insertLinkWithPlacement is synchronous (function, not async function), this requires making it async or restructuring the frontmatter path to properly await the promise.

Prompt for agents
The insertLinkWithPlacement function at src/utilityObsidian.ts:552 is currently synchronous (returns void), but the new inFrontmatter code path calls app.fileManager.processFrontMatter which is async and returns Promise<void>. The promise is silently discarded.

To fix this properly, you need to either:
1. Make insertLinkWithPlacement async (returning Promise<void>) and await processFrontMatter. Then update insertFileLinkToActiveView (line 734) to also be async and await the call. Then update all callers (CaptureChoiceEngine.insertCaptureLink at line 173, TemplateChoiceEngine around line 141) to await as well.
2. Or, separate the frontmatter insertion into its own async function and handle it at a higher level in the engine code, keeping insertLinkWithPlacement synchronous for editor-based placements.

Option 1 is simpler but has a wider blast radius. Option 2 preserves the synchronous contract for existing placement modes while properly handling the async frontmatter path.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}
Comment on lines +592 to +613

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing await on async processFrontMatter call.

The app.fileManager.processFrontMatter method returns a Promise<void>, but line 602 does not await it. This means the function returns before the frontmatter is actually written, which can cause race conditions or data loss if subsequent code assumes the frontmatter has been updated.

🐛 Proposed fix
-	if (mode === "inFrontmatter") {
-		if(!frontmatterProperty) {
+	if (mode === "inFrontmatter") {
+		if (!frontmatterProperty) {
 			const message = "Invalid frontmatter property: " + frontmatterProperty;
 			throw new Error(message);
 		}
 		const file = view.file;
-		if(!file) {
+		if (!file) {
 			const message = "Could not find file of active view";
 			throw new Error(message);
 		}
-		app.fileManager.processFrontMatter(file, (frontmatter) => {
+		await app.fileManager.processFrontMatter(file, (frontmatter) => {
 			if (frontmatter[frontmatterProperty] === undefined) {
-				frontmatter[frontmatterProperty] = []
+				frontmatter[frontmatterProperty] = [];
 			}
 			if (!Array.isArray(frontmatter[frontmatterProperty])) {
 				const message = "Could not add into non array property:" + frontmatterProperty;
-				throw new Error(message)
+				throw new Error(message);
 			}
-			frontmatter[frontmatterProperty].push(text)
-		})
-		return
+			frontmatter[frontmatterProperty].push(text);
+		});
+		return;
 	}

Note: This also requires making insertLinkWithPlacement an async function:

-export function insertLinkWithPlacement(
+export async function insertLinkWithPlacement(

And updating the call site at line 761 to await it.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utilityObsidian.ts` around lines 592 - 613, The processFrontMatter
callback is awaited incorrectly: call app.fileManager.processFrontMatter(...)
must be awaited so the frontmatter write completes before returning; update the
function containing this block (e.g., insertLinkWithPlacement) to be async,
prefix the processFrontMatter call with await, and update any callers (such as
the call at the site that previously invoked insertLinkWithPlacement) to await
insertLinkWithPlacement as well; ensure you still throw the same errors for
invalid frontmatterProperty and missing file and keep the existing logic inside
the callback unchanged.


//////////////////////////////////////////////////////////////////
// ALL OTHER MODES NEED EXPLICIT POSITION CALCULATION
Expand Down Expand Up @@ -735,7 +762,7 @@ export function insertFileLinkToActiveView(
app,
linkText,
linkOptions.placement,
{ requireActiveView: false },
{ requireActiveView: false, frontmatterProperty : linkOptions.frontmatterProperty },
);

return true;
Expand Down
Loading