feat(capture): fully support capture into canvas cards#1124
Merged
feat(capture): fully support capture into canvas cards#1124
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Canvas capture support: resolve active or configured Canvas targets, capture into Canvas text nodes or linked markdown files, merge content (top/bottom/insert-after), update engine/formatter/types/UI, add node picker UI and styles, tests, docs, and write-position migration/behavior updates. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as CaptureChoiceBuilder
participant Engine as CaptureChoiceEngine
participant Canvas as canvasCapture
participant Vault as Obsidian Vault
User->>UI: Trigger capture (with canvas target or active canvas)
UI->>Engine: Execute capture (includes nodeId or captureToActiveFile)
Engine->>Canvas: resolveConfigured/ActiveCanvasCaptureTarget(app, action)
Canvas->>Vault: Read .canvas file or active view
Canvas->>Canvas: Parse nodes, validate selection/target
Canvas-->>Engine: Return CanvasCaptureTarget (text/file)
alt Canvas Text Target
Engine->>Canvas: handleCanvasTextCapture(target, content, action)
Canvas->>Canvas: Validate action (top/bottom/insert-after)
Canvas->>Canvas: Merge content into node text
Canvas->>Vault: Persist updated .canvas
Canvas-->>Engine: Success
else Canvas File Target
Engine->>Vault: Write to linked markdown file (standard flow)
end
Engine-->>UI: Return result (notice/open file)
UI-->>User: Show success/notice
sequenceDiagram
actor User
participant UI as CaptureChoiceBuilder
participant Engine as CaptureChoiceEngine
participant Canvas as canvasCapture
participant Vault as Obsidian Vault
User->>UI: Trigger capture to active canvas card
UI->>Engine: Execute capture (captureToActiveFile=true)
Engine->>Canvas: resolveActiveCanvasCaptureTarget(app, action)
Canvas->>Vault: Get active canvas view and selection
Canvas->>Canvas: Validate single node, determine kind
Canvas-->>Engine: CanvasCaptureTarget
alt Active Canvas Text Node
Engine->>Canvas: handleCanvasTextCapture(target, content, action)
Canvas->>Canvas: Merge content based on action
Canvas->>Vault: Update in-memory canvas node (persist if needed)
Canvas-->>Engine: Success
else Active Canvas File Card
Engine->>Vault: Capture to linked markdown file
end
Engine-->>UI: Complete capture flow
UI-->>User: Confirm insertion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying quickadd with
|
| Latest commit: |
7885014
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://18eb60ab.quickadd.pages.dev |
| Branch Preview URL: | https://379-bug-cannot-insert-into-c.quickadd.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (7)
src/engine/canvasCapture.ts (1)
163-165:isTFileLikeduck-type check may produce false positives.Any object with
pathandextensionproperties will pass this check (e.g., a folder-like object with anextensionproperty added). SinceTAbstractFile(theTFoldersubclass) won't haveextension, this is probably safe in practice, but consider checkingtypeof file.extension === "string"for added robustness if desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine/canvasCapture.ts` around lines 163 - 165, The duck-typing in isTFileLike may return true for objects that merely have path and extension properties; update isTFileLike to also verify the type of extension (e.g., typeof file.extension === "string") and keep the existing null/undefined check so only genuine TFile-like objects pass; locate the isTFileLike function and add the typeof guard alongside the "path" and "extension" property checks.src/engine/canvasCapture.test.ts (1)
191-229: Consider asserting the full round-trip (resolve → get content → set content → verify persisted structure).The test verifies that
modifyis called with the right JSON, which is good. One optional improvement: you could also verify thatgetCanvasTextCaptureContentreturns the new value aftersetCanvasTextCaptureContentto confirm in-memory state is consistent. This would catch a bug where the file is persisted correctly but the in-memorynodeData.textisn't updated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine/canvasCapture.test.ts` around lines 191 - 229, Add a round-trip assertion: after calling setCanvasTextCaptureContent(app, target, "Updated") also call getCanvasTextCaptureContent(app, target) and assert it returns "Updated" (or the expected structure/value) to ensure in-memory nodeData.text was updated as well as the file persisted; use the existing target from resolveConfiguredCanvasCaptureTarget and the modified JSON parsing already present to locate where to add this extra expect.src/engine/CaptureChoiceEngine.ts (2)
286-307: Duplicated post-capture epilogue (notice → link → open file → templater cursor).Lines 286-307 replicate the same sequence found in
run()at Lines 207-229. If a fourth concern is added later (e.g., analytics, property vars), it must be updated in both places.Consider extracting a shared
postCaptureEpilogue(file, { wasNewFile, action, linkOptions })helper. Not blocking, but worth tracking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine/CaptureChoiceEngine.ts` around lines 286 - 307, The epilogue sequence (showSuccessNotice → insertFileLinkToActiveView → open file handling → jumpToNextTemplaterCursorIfPossible) is duplicated in CaptureChoiceEngine.run() and the block starting at the shown snippet; extract a helper (e.g., postCaptureEpilogue) that takes (file, { wasNewFile, action, linkOptions, fileOpening, choice }) and move the shared logic into that function, then call postCaptureEpilogue from both places; ensure the helper uses showSuccessNotice, insertFileLinkToActiveView when linkOptions.enabled, uses normalizeFileOpening and openExistingFileTab, awaits openFile when needed, and calls jumpToNextTemplaterCursorIfPossible so behavior is identical.
149-163: Type assertions onGetFileAndAddContentFnare brittle.Both
onFileExists(2-param signature) and theonCreateFileIfItDoesntExistwrapper are cast viaas GetFileAndAddContentFn. If either method's signature drifts, the compiler won't catch the mismatch.Consider making the adapter explicit or aligning the signatures so the cast isn't needed.
♻️ Suggested approach
- type GetFileAndAddContentFn = ( - path: string, - capture: string, - linkOptions?: AppendLinkOptions, - ) => Promise<{ file: TFile; newFileContent: string; captureContent: string }>; - let getFileAndAddContentFn: GetFileAndAddContentFn; const fileAlreadyExists = await this.fileExists(filePath); if (fileAlreadyExists) { - getFileAndAddContentFn = - this.onFileExists.bind(this) as GetFileAndAddContentFn; + getFileAndAddContentFn = (path, capture) => + this.onFileExists(path, capture); } else if (this.choice?.createFileIfItDoesntExist?.enabled) { - getFileAndAddContentFn = ((path, capture, _options) => - this.onCreateFileIfItDoesntExist(path, capture, linkOptions) - ) as GetFileAndAddContentFn; + getFileAndAddContentFn = (path, capture) => + this.onCreateFileIfItDoesntExist(path, capture, linkOptions);With lambda wrappers the return type is inferred and no cast is needed. The local type alias can be kept or dropped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine/CaptureChoiceEngine.ts` around lines 149 - 163, The current use of type assertions for GetFileAndAddContentFn is brittle—remove the casts and provide explicit adapter functions whose parameter and return signatures match the GetFileAndAddContentFn alias: e.g., create a small wrapper function that takes (path: string, capture: string, linkOptions?: AppendLinkOptions) and calls this.onFileExists or this.onCreateFileIfItDoesntExist with the proper arguments (or update those methods to accept the optional third param and return the exact Promise<{file: TFile; newFileContent: string; captureContent: string}>), then assign that adapter to getFileAndAddContentFn so the compiler verifies types for onFileExists and onCreateFileIfItDoesntExist without using as casts.src/formatters/captureChoiceFormatter-write-position.test.ts (1)
5-106: Consider extracting shared mocks to reduce boilerplate across test files.This file has ~100 lines of
vi.mockcalls that are likely duplicated across other test files (e.g.,canvasCapture.test.ts,captureAction.test.ts). A shared test helper (e.g.,tests/mocks/obsidianMocks.ts) could reduce maintenance burden.Not blocking for this PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/formatters/captureChoiceFormatter-write-position.test.ts` around lines 5 - 106, The test contains many repeated vi.mock declarations (e.g., modules "../utilityObsidian", "../gui/InputPrompt", "../gui/InputSuggester/inputSuggester", "../gui/GenericSuggester/genericSuggester", "../gui/VDateInputPrompt/VDateInputPrompt", "../utils/errorUtils", "../gui/MathModal", "../engine/SingleInlineScriptEngine", "../engine/SingleMacroEngine", "../engine/SingleTemplateEngine", "obsidian-dataview", "../main") which are duplicated in other tests; extract these into a single shared helper (e.g., tests/mocks/obsidianMocks.ts) that exports a setupMocks function which registers all those vi.mock stubs, then replace the block in captureChoiceFormatter-write-position.test.ts with an import and a call to that setup function so tests reuse the centralized mocks.src/gui/ChoiceBuilder/captureChoiceBuilder.ts (2)
183-198:getFiles()scans the entire vault — acceptable here but worth noting.
this.app.vault.getFiles()iterates all vault files to find.canvasfiles. For very large vaults this adds overhead on every settings dialog open. If performance becomes a concern, consider caching or lazily loading the canvas paths. Not blocking for now.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gui/ChoiceBuilder/captureChoiceBuilder.ts` around lines 183 - 198, The current code in captureChoiceBuilder builds canvasPaths by calling this.app.vault.getFiles() (used alongside getMarkdownFiles() and FILE_NAME_FORMAT_SYNTAX to form captureTargetSuggestions), which scans the whole vault on every dialog open; to fix, avoid repeated full-vault scans by caching or lazily computing canvasPaths: add a persistent cache on the CaptureChoiceBuilder instance (or a shared service) keyed to vault state and populate canvasPaths once (or compute on first open), and invalidate/update the cache on vault change events so captureTargetSuggestions uses the cached canvasPaths instead of calling getFiles() every time.
458-493:activeLeafis deprecated in recent Obsidian API versions.
this.app.workspace.activeLeafis marked as deprecated in the Obsidian API typings ("use of this field is discouraged"). While it continues to work, consider usingthis.app.workspace.getMostRecentLeaf()instead, which more explicitly targets the most recent leaf in the main area.However, since the Canvas view type is not exported from the Obsidian API, the current inline type assertion is a pragmatic necessity—alternatives like
getMostRecentLeaf()would require the same approach. If this pattern recurs, consider extracting a reusable helper function to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gui/ChoiceBuilder/captureChoiceBuilder.ts` around lines 458 - 493, Replace usage of the deprecated this.app.workspace.activeLeaf in getActiveCanvasSelectionNodeIdForPath with this.app.workspace.getMostRecentLeaf(), keeping the same inline view typing and logic; update the local variable name (e.g., activeLeaf -> recentLeaf) and use recentLeaf?.view where the code currently references activeLeaf?.view, preserving calls to normalizeVaultPath, view.getViewType, view.file?.path, and view.canvas?.selection; optionally extract this pattern to a small helper that returns a canvas-typed view to reduce duplication across the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/engine/canvasCapture.test.ts`:
- Around line 191-229: Add a round-trip assertion: after calling
setCanvasTextCaptureContent(app, target, "Updated") also call
getCanvasTextCaptureContent(app, target) and assert it returns "Updated" (or the
expected structure/value) to ensure in-memory nodeData.text was updated as well
as the file persisted; use the existing target from
resolveConfiguredCanvasCaptureTarget and the modified JSON parsing already
present to locate where to add this extra expect.
In `@src/engine/canvasCapture.ts`:
- Around line 163-165: The duck-typing in isTFileLike may return true for
objects that merely have path and extension properties; update isTFileLike to
also verify the type of extension (e.g., typeof file.extension === "string") and
keep the existing null/undefined check so only genuine TFile-like objects pass;
locate the isTFileLike function and add the typeof guard alongside the "path"
and "extension" property checks.
In `@src/engine/CaptureChoiceEngine.ts`:
- Around line 286-307: The epilogue sequence (showSuccessNotice →
insertFileLinkToActiveView → open file handling →
jumpToNextTemplaterCursorIfPossible) is duplicated in CaptureChoiceEngine.run()
and the block starting at the shown snippet; extract a helper (e.g.,
postCaptureEpilogue) that takes (file, { wasNewFile, action, linkOptions,
fileOpening, choice }) and move the shared logic into that function, then call
postCaptureEpilogue from both places; ensure the helper uses showSuccessNotice,
insertFileLinkToActiveView when linkOptions.enabled, uses normalizeFileOpening
and openExistingFileTab, awaits openFile when needed, and calls
jumpToNextTemplaterCursorIfPossible so behavior is identical.
- Around line 149-163: The current use of type assertions for
GetFileAndAddContentFn is brittle—remove the casts and provide explicit adapter
functions whose parameter and return signatures match the GetFileAndAddContentFn
alias: e.g., create a small wrapper function that takes (path: string, capture:
string, linkOptions?: AppendLinkOptions) and calls this.onFileExists or
this.onCreateFileIfItDoesntExist with the proper arguments (or update those
methods to accept the optional third param and return the exact Promise<{file:
TFile; newFileContent: string; captureContent: string}>), then assign that
adapter to getFileAndAddContentFn so the compiler verifies types for
onFileExists and onCreateFileIfItDoesntExist without using as casts.
In `@src/formatters/captureChoiceFormatter-write-position.test.ts`:
- Around line 5-106: The test contains many repeated vi.mock declarations (e.g.,
modules "../utilityObsidian", "../gui/InputPrompt",
"../gui/InputSuggester/inputSuggester",
"../gui/GenericSuggester/genericSuggester",
"../gui/VDateInputPrompt/VDateInputPrompt", "../utils/errorUtils",
"../gui/MathModal", "../engine/SingleInlineScriptEngine",
"../engine/SingleMacroEngine", "../engine/SingleTemplateEngine",
"obsidian-dataview", "../main") which are duplicated in other tests; extract
these into a single shared helper (e.g., tests/mocks/obsidianMocks.ts) that
exports a setupMocks function which registers all those vi.mock stubs, then
replace the block in captureChoiceFormatter-write-position.test.ts with an
import and a call to that setup function so tests reuse the centralized mocks.
In `@src/gui/ChoiceBuilder/captureChoiceBuilder.ts`:
- Around line 183-198: The current code in captureChoiceBuilder builds
canvasPaths by calling this.app.vault.getFiles() (used alongside
getMarkdownFiles() and FILE_NAME_FORMAT_SYNTAX to form
captureTargetSuggestions), which scans the whole vault on every dialog open; to
fix, avoid repeated full-vault scans by caching or lazily computing canvasPaths:
add a persistent cache on the CaptureChoiceBuilder instance (or a shared
service) keyed to vault state and populate canvasPaths once (or compute on first
open), and invalidate/update the cache on vault change events so
captureTargetSuggestions uses the cached canvasPaths instead of calling
getFiles() every time.
- Around line 458-493: Replace usage of the deprecated
this.app.workspace.activeLeaf in getActiveCanvasSelectionNodeIdForPath with
this.app.workspace.getMostRecentLeaf(), keeping the same inline view typing and
logic; update the local variable name (e.g., activeLeaf -> recentLeaf) and use
recentLeaf?.view where the code currently references activeLeaf?.view,
preserving calls to normalizeVaultPath, view.getViewType, view.file?.path, and
view.canvas?.selection; optionally extract this pattern to a small helper that
returns a canvas-typed view to reduce duplication across the codebase.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/docs/Choices/CaptureChoice.mdsrc/engine/CaptureChoiceEngine.tssrc/engine/canvasCapture.test.tssrc/engine/canvasCapture.tssrc/engine/captureAction.test.tssrc/engine/captureAction.tssrc/formatters/captureChoiceFormatter-write-position.test.tssrc/formatters/captureChoiceFormatter.tssrc/gui/ChoiceBuilder/captureChoiceBuilder.tssrc/styles.csssrc/types/choices/CaptureChoice.test.tssrc/types/choices/CaptureChoice.tssrc/types/choices/ICaptureChoice.ts
chhoumann
changed the title
chhoumann
force-pushed
the
379-bug-cannot-insert-into-canvas-card
branch
from
70bf684 to
d852c29
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
chhoumann
changed the title
This was referenced Feb 27, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
Mar 5, 2026
# [2.12.0](2.11.0...2.12.0) (2026-03-05) ### Bug Fixes * **capture:** preserve canvas tab indentation on configured writes ([#1125](#1125)) ([0a1578e](0a1578e)) * disallow capture targets with .base extension ([cb39ed4](cb39ed4)) * **field-suggestions:** opt-in inline values from fenced code blocks ([#1128](#1128)) ([8597905](8597905)) * **gui:** preserve modal edit position during settings reload ([#1132](#1132)) ([11bda19](11bda19)) * **gui:** reduce ai settings modal reload churn ([#1134](#1134)) ([ae0f7a1](ae0f7a1)) * **gui:** reduce choice builder reload churn ([#1136](#1136)) ([818272a](818272a)) * **gui:** reduce macro settings modal reload churn ([#1135](#1135)) ([a1a6271](a1a6271)) * harden existing-tab matching and document issue workflow ([#1108](#1108)) ([7b12d3b](7b12d3b)) * make template path resolution deterministic ([3297d54](3297d54)) * normalize capture title for non-markdown targets ([964d672](964d672)) * preserve capture-format spacing for insert-at-end ([#1119](#1119)) ([8bb8ed4](8bb8ed4)) * preserve explicit capture target file extensions ([57e43ff](57e43ff)) * preserve insert-at-end order for non-newline captures ([#1120](#1120)) ([e7cbbf2](e7cbbf2)) * resolve template file-name paths without duplicate default folder ([7bfd41b](7bfd41b)) * resolve vault-relative template paths using root folders ([81216de](81216de)) ### Features * add AI request logging API and reduce assistant log noise ([#1110](#1110)) ([2c36800](2c36800)) * automate docs version snapshot during release ([#1111](#1111)) ([1571846](1571846)) * **capture:** fully support capture into canvas cards ([#1124](#1124)) ([a53f889](a53f889)) * **cli:** add native QuickAdd Obsidian CLI handlers ([#1129](#1129)) ([8102d47](8102d47)) * **format:** support mapped VALUE suggester display text ([#1127](#1127)) ([b8ec56c](b8ec56c)) * **macro:** add editor cursor navigation commands ([101d5f6](101d5f6)) * support .base template files for template choices ([11e6490](11e6490)) ### Reverts * **gui:** remove modal reload refactor series ([#1137](#1137)) ([3ba1a73](3ba1a73)), closes [#1136](#1136) [#1135](#1135) [#1134](#1134) [#1133](#1133) [#1132](#1132)
|
🎉 This PR is included in version 2.12.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This was referenced Apr 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add first-class Canvas capture targeting and fix write-position behavior for both Canvas and non-Canvas capture paths.
This addresses the long-standing Canvas capture failure in #379 and expands Capture so it can target a specific node in a specific
.canvasfile instead of only depending on active selection context.What changed
captureTo.canvaspath + explicitcaptureToCanvasNodeId..canvasfiles are suggested inFile path / format.Target canvas nodeis only shown when targeting a.canvaspath.Use node,Copy ID,Use selected in open canvas, andOpen target canvas.bottomwrite mode.prependstates that previously represented bottom writes.Alternatives considered
Verification
bun run test src/formatters/captureChoiceFormatter-write-position.test.ts src/types/choices/CaptureChoice.test.ts src/engine/captureAction.test.tsBottom of fileappends to EOF..mdand.canvastargets honor top/bottom modes.Notes for review
docs/docs/Choices/CaptureChoice.mduntil a stronger standalone example is ready.Fixes #379
Refs #367
Refs #637
Refs #479
Summary by CodeRabbit
New Features
Behavioral Changes
Documentation
Tests
Style