Skip to content

Commit c6cc64a

Browse files
authored
Merge pull request #1098 from chhoumann/fix/issue-729-capture-folder
fix: improve capture folder resolution and missing-file notice
2 parents 73918f2 + c366fec commit c6cc64a

5 files changed

Lines changed: 193 additions & 30 deletions

File tree

src/engine/CaptureChoiceEngine.notice.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,4 +240,48 @@ describe("CaptureChoiceEngine cancellation notices", () => {
240240
"Capture execution aborted: Target file missing",
241241
);
242242
});
243+
244+
it("shows a notice when the target file is missing and create is disabled", async () => {
245+
settingsStore.setState({
246+
...settingsStore.getState(),
247+
showInputCancellationNotification: false,
248+
});
249+
250+
const app = {
251+
vault: {
252+
adapter: {
253+
exists: vi.fn(async () => false),
254+
},
255+
getAbstractFileByPath: vi.fn(),
256+
modify: vi.fn(),
257+
create: vi.fn(),
258+
},
259+
workspace: {
260+
getActiveFile: vi.fn(() => null),
261+
},
262+
fileManager: {
263+
getNewFileParent: vi.fn(() => ({ path: "" })),
264+
},
265+
} as unknown as App;
266+
267+
const plugin = { settings: settingsStore.getState() } as any;
268+
const choiceExecutor: IChoiceExecutor = {
269+
execute: vi.fn(),
270+
variables: new Map<string, unknown>(),
271+
};
272+
273+
const engine = new CaptureChoiceEngine(
274+
app,
275+
plugin,
276+
createCaptureChoice(),
277+
choiceExecutor,
278+
);
279+
280+
await engine.run();
281+
282+
expect(noticeClass.instances).toHaveLength(1);
283+
expect(noticeClass.instances[0]?.message).toContain(
284+
"Capture execution aborted: Target file missing",
285+
);
286+
});
243287
});

src/engine/CaptureChoiceEngine.selection.test.ts

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { App } from "obsidian";
33
import { CaptureChoiceEngine } from "./CaptureChoiceEngine";
44
import type ICaptureChoice from "../types/choices/ICaptureChoice";
55
import type { IChoiceExecutor } from "../IChoiceExecutor";
6-
import { openFile } from "../utilityObsidian";
6+
import { isFolder, openFile } from "../utilityObsidian";
77

88
const { setUseSelectionAsCaptureValueMock } = vi.hoisted(() => ({
99
setUseSelectionAsCaptureValueMock: vi.fn(),
@@ -74,6 +74,7 @@ const createApp = () =>
7474
adapter: {
7575
exists: vi.fn(async () => false),
7676
},
77+
getAbstractFileByPath: vi.fn(() => null),
7778
},
7879
workspace: {
7980
getActiveFile: vi.fn(() => null),
@@ -209,3 +210,58 @@ describe("CaptureChoiceEngine selection-as-value resolution", () => {
209210
);
210211
});
211212
});
213+
214+
describe("CaptureChoiceEngine capture target resolution", () => {
215+
beforeEach(() => {
216+
vi.mocked(isFolder).mockReset();
217+
});
218+
219+
it("treats folder path without trailing slash as folder when folder exists", () => {
220+
const app = createApp();
221+
vi.mocked(isFolder).mockReturnValue(true);
222+
223+
const engine = new CaptureChoiceEngine(
224+
app,
225+
{ settings: { useSelectionAsCaptureValue: false } } as any,
226+
createChoice({ captureTo: "journals" }),
227+
createExecutor(),
228+
);
229+
230+
const result = (engine as any).resolveCaptureTarget("journals");
231+
232+
expect(result).toEqual({ kind: "folder", folder: "journals" });
233+
});
234+
235+
it("treats trailing slash as folder even when folder does not exist", () => {
236+
const app = createApp();
237+
vi.mocked(isFolder).mockReturnValue(false);
238+
239+
const engine = new CaptureChoiceEngine(
240+
app,
241+
{ settings: { useSelectionAsCaptureValue: false } } as any,
242+
createChoice({ captureTo: "journals/" }),
243+
createExecutor(),
244+
);
245+
246+
const result = (engine as any).resolveCaptureTarget("journals/");
247+
248+
expect(result).toEqual({ kind: "folder", folder: "journals" });
249+
});
250+
251+
it("treats folder path as file when a file exists", () => {
252+
const app = createApp();
253+
vi.mocked(isFolder).mockReturnValue(true);
254+
vi.mocked(app.vault.getAbstractFileByPath).mockReturnValue({} as any);
255+
256+
const engine = new CaptureChoiceEngine(
257+
app,
258+
{ settings: { useSelectionAsCaptureValue: false } } as any,
259+
createChoice({ captureTo: "journals" }),
260+
createExecutor(),
261+
);
262+
263+
const result = (engine as any).resolveCaptureTarget("journals");
264+
265+
expect(result).toEqual({ kind: "file", path: "journals" });
266+
});
267+
});

src/engine/CaptureChoiceEngine.ts

Lines changed: 71 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
import { isCancellationError, reportError } from "../utils/errorUtils";
3232
import { normalizeFileOpening } from "../utils/fileOpeningDefaults";
3333
import { QuickAddChoiceEngine } from "./QuickAddChoiceEngine";
34+
import { ChoiceAbortError } from "../errors/ChoiceAbortError";
3435
import { MacroAbortError } from "../errors/MacroAbortError";
3536
import { SingleTemplateEngine } from "./SingleTemplateEngine";
3637
import { getCaptureAction, type CaptureAction } from "./captureAction";
@@ -130,12 +131,11 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine {
130131
getFileAndAddContentFn = ((path, capture, _options) =>
131132
this.onCreateFileIfItDoesntExist(path, capture, linkOptions)
132133
) as typeof this.onCreateFileIfItDoesntExist;
133-
} else {
134-
log.logWarning(
135-
`The file ${filePath} does not exist and "Create file if it doesn't exist" is disabled.`,
136-
);
137-
return;
138-
}
134+
} else {
135+
throw new ChoiceAbortError(
136+
`Target file missing: ${filePath}. Enable "Create file if it doesn't exist" or choose an existing file.`,
137+
);
138+
}
139139

140140
const { file, newFileContent, captureContent } =
141141
await getFileAndAddContentFn(filePath, content);
@@ -258,27 +258,75 @@ export class CaptureChoiceEngine extends QuickAddChoiceEngine {
258258
}
259259

260260
const captureTo = this.choice.captureTo;
261-
const formattedCaptureTo = await this.formatFilePath(captureTo);
262-
263-
// Removing the trailing slash from the capture to path because otherwise isFolder will fail
264-
// to get the folder.
265-
const folderPath = formattedCaptureTo.replace(/^\/$|\/\.md$|^\.md$/, "");
266-
// Empty string means we suggest to capture anywhere in the vault.
267-
const captureAnywhereInVault = folderPath === "";
268-
const shouldCaptureToFolder =
269-
captureAnywhereInVault || isFolder(this.app, folderPath);
270-
const shouldCaptureWithTag = formattedCaptureTo.startsWith("#");
271-
272-
if (shouldCaptureToFolder) {
273-
return this.selectFileInFolder(folderPath, captureAnywhereInVault);
261+
const formattedCaptureTo = await this.formatter.formatFileName(
262+
captureTo,
263+
this.choice.name,
264+
);
265+
const resolution = this.resolveCaptureTarget(formattedCaptureTo);
266+
267+
switch (resolution.kind) {
268+
case "vault":
269+
return this.selectFileInFolder("", true);
270+
case "tag":
271+
return this.selectFileWithTag(resolution.tag);
272+
case "folder":
273+
return this.selectFileInFolder(resolution.folder, false);
274+
case "file":
275+
return this.normalizeMarkdownFilePath("", resolution.path);
276+
}
277+
}
278+
279+
private resolveCaptureTarget(
280+
formattedCaptureTo: string,
281+
):
282+
| { kind: "vault" }
283+
| { kind: "tag"; tag: string }
284+
| { kind: "folder"; folder: string }
285+
| { kind: "file"; path: string } {
286+
// Resolution order:
287+
// 1) empty => vault picker
288+
// 2) #tag => tag picker
289+
// 3) trailing "/" => folder picker (explicit)
290+
// 4) ".md" => file
291+
// 5) ambiguous => folder if it exists and no same-name file exists; else file
292+
const normalizedCaptureTo = this.stripLeadingSlash(
293+
formattedCaptureTo.trim(),
294+
);
295+
296+
if (normalizedCaptureTo === "") {
297+
return { kind: "vault" };
298+
}
299+
300+
if (normalizedCaptureTo.startsWith("#")) {
301+
return {
302+
kind: "tag",
303+
tag: normalizedCaptureTo.replace(/\.md$/, ""),
304+
};
274305
}
275306

276-
if (shouldCaptureWithTag) {
277-
const tag = formattedCaptureTo.replace(/\.md$/, "");
278-
return this.selectFileWithTag(tag);
307+
const endsWithSlash = normalizedCaptureTo.endsWith("/");
308+
const folderPath = normalizedCaptureTo.replace(/\/+$/, "");
309+
310+
if (endsWithSlash) {
311+
return { kind: "folder", folder: folderPath };
312+
}
313+
314+
if (normalizedCaptureTo.endsWith(".md")) {
315+
return { kind: "file", path: normalizedCaptureTo };
316+
}
317+
318+
// Guard against ambiguity where a folder and file share the same name.
319+
const fileCandidatePath = this.normalizeMarkdownFilePath("", folderPath);
320+
const fileCandidate = this.app.vault.getAbstractFileByPath(
321+
fileCandidatePath,
322+
);
323+
const fileExists = !!fileCandidate;
324+
325+
if (isFolder(this.app, folderPath) && !fileExists) {
326+
return { kind: "folder", folder: folderPath };
279327
}
280328

281-
return formattedCaptureTo;
329+
return { kind: "file", path: normalizedCaptureTo };
282330
}
283331

284332
private async selectFileInFolder(

src/errors/ChoiceAbortError.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { MacroAbortError } from "./MacroAbortError";
2+
3+
export class ChoiceAbortError extends MacroAbortError {}

src/gui/ChoiceBuilder/captureChoiceBuilder.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
normalizeAppendLinkOptions,
1616
placementSupportsEmbed,
1717
} from "../../types/linkPlacement";
18+
import { getAllFolderPathsInVault } from "../../utilityObsidian";
1819
import { createValidatedInput } from "../components/validatedInput";
1920
import { FormatSyntaxSuggester } from "../suggesters/formatSyntaxSuggester";
2021
import { ChoiceBuilder } from "./choiceBuilder";
@@ -125,7 +126,9 @@ export class CaptureChoiceBuilder extends ChoiceBuilder {
125126
private addCapturedToSetting() {
126127
new Setting(this.contentEl)
127128
.setName("Capture to")
128-
.setDesc("Target file path. Supports format syntax.");
129+
.setDesc(
130+
"Vault-relative path. Supports format syntax (use trailing '/' for folders).",
131+
);
129132

130133
const captureToContainer: HTMLDivElement =
131134
this.contentEl.createDiv("captureToContainer");
@@ -157,7 +160,9 @@ export class CaptureChoiceBuilder extends ChoiceBuilder {
157160

158161
new Setting(captureToFileContainer)
159162
.setName("File path / format")
160-
.setDesc("Choose a file or use format syntax (e.g., {{DATE}})");
163+
.setDesc(
164+
"Choose a file, folder, or format syntax (e.g., {{DATE}})",
165+
);
161166

162167
const displayFormatter: FileNameDisplayFormatter =
163168
new FileNameDisplayFormatter(this.app, this.plugin);
@@ -168,10 +173,17 @@ export class CaptureChoiceBuilder extends ChoiceBuilder {
168173
formatDisplay.setAttr("aria-live", "polite");
169174
formatDisplay.textContent = "Loading preview…";
170175

171-
const markdownFilesAndFormatSyntax = [
172-
...this.app.vault.getMarkdownFiles().map((f) => f.path),
173-
...FILE_NAME_FORMAT_SYNTAX,
174-
];
176+
const folderPaths = getAllFolderPathsInVault(this.app)
177+
.filter((path) => path.length > 0)
178+
.map((path) => (path.endsWith("/") ? path : `${path}/`));
179+
180+
const markdownFilesAndFormatSyntax = Array.from(
181+
new Set([
182+
...folderPaths,
183+
...this.app.vault.getMarkdownFiles().map((f) => f.path),
184+
...FILE_NAME_FORMAT_SYNTAX,
185+
]),
186+
);
175187

176188
createValidatedInput({
177189
app: this.app,

0 commit comments

Comments
 (0)