Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds centralized file-opening defaults and normalization, a migration to backfill missing fileOpening settings (including legacy mappings), traversal helpers for nested choices/macros, updates constructors/engines to use normalization, and corresponding tests and registration in the migrations map. Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin
participant Migration as backfillFileOpeningDefaults
participant Walker as walkAllChoices
participant Visitor as choice visitor
participant Legacy as file-opening legacy helpers
participant Storage as plugin.saveSettings
participant Logger
Plugin->>Migration: migrate(plugin)
Migration->>Logger: log "starting migration"
Migration->>Walker: walkAllChoices(plugin, visitor)
Walker->>Visitor: visit(choice)
Visitor->>Legacy: coerce legacy fields / createFileOpeningFromLegacy
Visitor->>Visitor: normalizeFileOpening and set choice.fileOpening
Visitor-->>Migration: indicate changed (increment)
Migration->>Storage: plugin.saveSettings()
Migration->>Logger: log "completed" with migratedCount
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (5)**/*.{ts,tsx,mts,mjs,js,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,svelte}📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/{engine,services,utils}/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/migrations/backfillFileOpeningDefaults.test.ts:
- Around line 15-191: The test fixtures (captureLegacy, captureMissing,
templatePartial, nestedCapture, nestedTemplate, conditionalCapture,
conditionalElseTemplate, legacyMacroCapture, multiChoice, macroWithNested,
macroWithConditional) are plain objects that the migration mutates by adding
fileOpening, causing TS2339; fix by giving these fixture constants a looser type
(e.g., declare each as any or cast them to a shared interface with an optional
fileOpening like Partial<Choice>), or type the choices array as any[] on the
plugin settings so TypeScript allows dynamic property addition; update the
declarations used by backfillFileOpeningDefaults.migrate (and the plugin object)
accordingly so the compiler accepts the migration mutations.
In @src/migrations/backfillFileOpeningDefaults.ts:
- Around line 30-34: The intermediate cast to Partial<FileOpeningSettings> (the
variable fileOpening) narrows type information and causes a TypeScript error
when calling normalizeFileOpening; remove the intermediate variable/cast and
pass templateOrCaptureChoice.fileOpening directly into normalizeFileOpening so
TypeScript can preserve TExtras generics and infer correctly (apply the same
change for the other occurrence around the second mention at lines ~54).
In @src/utils/fileOpeningDefaults.ts:
- Around line 17-48: The normalizeFileOpening function contains an unused
options parameter and a dead code branch controlled by fillMissingOnly; remove
the options parameter (and any mention of fillMissingOnly) from the signature of
normalizeFileOpening, delete the branch that returns the spread of
DEFAULT_FILE_OPENING when fillMissingOnly is false, and simplify the
implementation to always merge/fill missing fields from DEFAULT_FILE_OPENING
(keep references to DEFAULT_FILE_OPENING and the normalized variable and the
existing null checks for location, direction, mode, focus); update any
JSDoc/typing to reflect the new signature (fileOpening?:
(Partial<FileOpeningSettings> & TExtras) | null) & ensure the function still
returns FileOpeningSettings & TExtras.
🧹 Nitpick comments (4)
src/engine/TemplateChoiceEngine.ts (1)
179-180: Simplify focus extraction.The
normalizeFileOpeningfunction already guarantees thatfocusis defined (defaults totrueif missing). The nullish coalescing on line 180 is redundant.♻️ Suggested simplification
const fileOpening = normalizeFileOpening(this.choice.fileOpening); -const focus = fileOpening.focus ?? true; +const focus = fileOpening.focus;src/engine/CaptureChoiceEngine.ts (1)
191-192: Simplify focus extraction.The
normalizeFileOpeningfunction already guarantees thatfocusis defined (defaults totrueif missing). The nullish coalescing on line 192 is redundant.♻️ Suggested simplification
const fileOpening = normalizeFileOpening(this.choice.fileOpening); -const focus = fileOpening.focus ?? true; +const focus = fileOpening.focus;src/migrations/backfillFileOpeningDefaults.ts (1)
24-57: Consider simplifying the conditional logic.The current logic has three cases:
- No defaults needed → return early
- No fileOpening + legacy fields → create from legacy
- Everything else → normalize
The else branch at line 54 catches both "partial fileOpening exists" and "no fileOpening + no legacy fields". In the latter case, you're normalizing
undefined, which works but could be clearer.🔄 Optional refactor for clarity
Consider making the intent more explicit:
if (!needsDefaults) return; - if (!fileOpening && legacyTab) { + if (legacyTab && !fileOpening) { templateOrCaptureChoice.fileOpening = createFileOpeningFromLegacy( legacyTab, legacyMode, ); - } else { + } else if (fileOpening) { templateOrCaptureChoice.fileOpening = normalizeFileOpening(fileOpening); + } else { + // No fileOpening and no legacy fields - create with all defaults + templateOrCaptureChoice.fileOpening = normalizeFileOpening(null); }This makes it explicit that we're creating defaults from scratch in the final case, though the current code works correctly too.
src/migrations/helpers/choice-traversal.ts (1)
12-14: Consider reusing the existingisMultiChoicehelper.An
isMultiChoicefunction already exists atsrc/migrations/helpers/isMultiChoice.tswith more comprehensive validation. Consider importing and reusing it instead of defining a new type guard here.♻️ Proposed refactor to reuse existing helper
+import { isMultiChoice } from "./isMultiChoice"; + -function isMultiChoice(choice: IChoice): choice is MultiChoice { - return choice.type === "Multi"; -}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/engine/CaptureChoiceEngine.notice.test.tssrc/engine/CaptureChoiceEngine.selection.test.tssrc/engine/CaptureChoiceEngine.template-property-types.test.tssrc/engine/CaptureChoiceEngine.tssrc/engine/MacroChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.tssrc/gui/ChoiceBuilder/choiceBuilder.tssrc/migrations/backfillFileOpeningDefaults.test.tssrc/migrations/backfillFileOpeningDefaults.tssrc/migrations/helpers/choice-traversal.tssrc/migrations/helpers/file-opening-legacy.tssrc/migrations/migrate.tssrc/migrations/migrateFileOpeningSettings.tssrc/settings.tssrc/types/choices/CaptureChoice.tssrc/types/choices/TemplateChoice.tssrc/utils/fileOpeningDefaults.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,mts,mjs,js,json}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,mts,mjs,js,json}: Use tab indentation with width 2 in TypeScript and configuration files (enforced by Biome).
Follow an 80-character line guide (enforced by Biome).
Files:
src/types/choices/CaptureChoice.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.tssrc/migrations/helpers/choice-traversal.tssrc/engine/CaptureChoiceEngine.selection.test.tssrc/utils/fileOpeningDefaults.tssrc/gui/ChoiceBuilder/choiceBuilder.tssrc/migrations/backfillFileOpeningDefaults.test.tssrc/migrations/migrateFileOpeningSettings.tssrc/engine/MacroChoiceEngine.notice.test.tssrc/engine/CaptureChoiceEngine.template-property-types.test.tssrc/migrations/backfillFileOpeningDefaults.tssrc/types/choices/TemplateChoice.tssrc/settings.tssrc/engine/CaptureChoiceEngine.tssrc/migrations/helpers/file-opening-legacy.tssrc/migrations/migrate.tssrc/engine/CaptureChoiceEngine.notice.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use camelCase for variables and functions in TypeScript.
Prefer type-only imports in TypeScript.
Files:
src/types/choices/CaptureChoice.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.tssrc/migrations/helpers/choice-traversal.tssrc/engine/CaptureChoiceEngine.selection.test.tssrc/utils/fileOpeningDefaults.tssrc/gui/ChoiceBuilder/choiceBuilder.tssrc/migrations/backfillFileOpeningDefaults.test.tssrc/migrations/migrateFileOpeningSettings.tssrc/engine/MacroChoiceEngine.notice.test.tssrc/engine/CaptureChoiceEngine.template-property-types.test.tssrc/migrations/backfillFileOpeningDefaults.tssrc/types/choices/TemplateChoice.tssrc/settings.tssrc/engine/CaptureChoiceEngine.tssrc/migrations/helpers/file-opening-legacy.tssrc/migrations/migrate.tssrc/engine/CaptureChoiceEngine.notice.test.ts
**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components.
Files:
src/types/choices/CaptureChoice.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.tssrc/migrations/helpers/choice-traversal.tssrc/engine/CaptureChoiceEngine.selection.test.tssrc/utils/fileOpeningDefaults.tssrc/gui/ChoiceBuilder/choiceBuilder.tssrc/migrations/backfillFileOpeningDefaults.test.tssrc/migrations/migrateFileOpeningSettings.tssrc/engine/MacroChoiceEngine.notice.test.tssrc/engine/CaptureChoiceEngine.template-property-types.test.tssrc/migrations/backfillFileOpeningDefaults.tssrc/types/choices/TemplateChoice.tssrc/settings.tssrc/engine/CaptureChoiceEngine.tssrc/migrations/helpers/file-opening-legacy.tssrc/migrations/migrate.tssrc/engine/CaptureChoiceEngine.notice.test.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Route logging through the
loggerutilities for consistent output.
Files:
src/types/choices/CaptureChoice.tssrc/engine/TemplateChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.tssrc/migrations/helpers/choice-traversal.tssrc/engine/CaptureChoiceEngine.selection.test.tssrc/utils/fileOpeningDefaults.tssrc/gui/ChoiceBuilder/choiceBuilder.tssrc/migrations/backfillFileOpeningDefaults.test.tssrc/migrations/migrateFileOpeningSettings.tssrc/engine/MacroChoiceEngine.notice.test.tssrc/engine/CaptureChoiceEngine.template-property-types.test.tssrc/migrations/backfillFileOpeningDefaults.tssrc/types/choices/TemplateChoice.tssrc/settings.tssrc/engine/CaptureChoiceEngine.tssrc/migrations/helpers/file-opening-legacy.tssrc/migrations/migrate.tssrc/engine/CaptureChoiceEngine.notice.test.ts
src/{engine,services,utils}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Structure production code so Obsidian dependencies are injected behind interfaces to enable unit testing without loading real Obsidian modules.
Files:
src/engine/TemplateChoiceEngine.notice.test.tssrc/engine/TemplateChoiceEngine.tssrc/engine/CaptureChoiceEngine.selection.test.tssrc/utils/fileOpeningDefaults.tssrc/engine/MacroChoiceEngine.notice.test.tssrc/engine/CaptureChoiceEngine.template-property-types.test.tssrc/engine/CaptureChoiceEngine.tssrc/engine/CaptureChoiceEngine.notice.test.ts
🧠 Learnings (4)
📚 Learning: 2025-12-21T07:54:34.875Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T07:54:34.875Z
Learning: Applies to src/{engine,services,utils}/**/*.{ts,tsx} : Structure production code so Obsidian dependencies are injected behind interfaces to enable unit testing without loading real Obsidian modules.
Applied to files:
src/engine/CaptureChoiceEngine.selection.test.ts
📚 Learning: 2025-12-21T07:54:34.875Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T07:54:34.875Z
Learning: Applies to tests/**/*.{ts,tsx} : Unit tests should target pure logic and swap in adapters or use `tests/obsidian-stub.ts` for Obsidian dependencies.
Applied to files:
src/engine/CaptureChoiceEngine.selection.test.ts
📚 Learning: 2025-12-21T07:54:34.875Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T07:54:34.875Z
Learning: Add regression coverage for bug fixes in tests.
Applied to files:
src/migrations/backfillFileOpeningDefaults.test.ts
📚 Learning: 2025-12-21T07:54:34.875Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T07:54:34.875Z
Learning: Applies to **/*.{ts,tsx} : Prefer type-only imports in TypeScript.
Applied to files:
src/types/choices/TemplateChoice.ts
🧬 Code graph analysis (9)
src/types/choices/CaptureChoice.ts (1)
src/utils/fileOpeningDefaults.ts (1)
normalizeFileOpening(17-48)
src/engine/TemplateChoiceEngine.ts (2)
src/utils/fileOpeningDefaults.ts (1)
normalizeFileOpening(17-48)src/utilityObsidian.ts (2)
openExistingFileTab(874-899)openFile(790-868)
src/migrations/helpers/choice-traversal.ts (7)
src/types/choices/IChoice.ts (1)
IChoice(3-10)src/migrations/helpers/isMultiChoice.ts (1)
isMultiChoice(3-14)src/types/choices/MultiChoice.ts (1)
MultiChoice(5-23)src/types/choices/IMacroChoice.ts (1)
IMacroChoice(4-7)src/types/macros/ICommand.ts (1)
ICommand(3-7)src/types/macros/Conditional/IConditionalCommand.ts (1)
IConditionalCommand(4-8)src/types/macros/QuickCommands/INestedChoiceCommand.ts (1)
INestedChoiceCommand(4-6)
src/engine/CaptureChoiceEngine.selection.test.ts (2)
src/types/choices/ICaptureChoice.ts (1)
ICaptureChoice(7-56)src/engine/CaptureChoiceEngine.ts (1)
CaptureChoiceEngine(41-537)
src/gui/ChoiceBuilder/choiceBuilder.ts (1)
src/utils/fileOpeningDefaults.ts (2)
normalizeFileOpening(17-48)FileOpeningSettings(3-8)
src/migrations/migrateFileOpeningSettings.ts (2)
src/migrations/helpers/file-opening-legacy.ts (1)
coerceLegacyOpenFileInNewTab(12-30)src/migrations/helpers/choice-traversal.ts (1)
walkAllChoices(77-90)
src/types/choices/TemplateChoice.ts (1)
src/utils/fileOpeningDefaults.ts (1)
normalizeFileOpening(17-48)
src/engine/CaptureChoiceEngine.ts (3)
src/utils/fileOpeningDefaults.ts (1)
normalizeFileOpening(17-48)src/utilityObsidian.ts (2)
openExistingFileTab(874-899)openFile(790-868)tests/obsidian-stub.ts (1)
openFile(363-363)
src/migrations/helpers/file-opening-legacy.ts (1)
src/utils/fileOpeningDefaults.ts (2)
FileOpeningSettings(3-8)normalizeFileOpening(17-48)
🪛 GitHub Actions: Build With Lint
src/migrations/backfillFileOpeningDefaults.test.ts
[error] 144-144: TypeScript error TS2339: Property 'fileOpening' does not exist on type '{ id: string; name: string; type: string; openFileInNewTab: { enabled: boolean; direction: string; focus: boolean; }; openFileInMode: string; }'. (During bun run build-with-lint: tsc -noEmit -skipLibCheck)
🪛 GitHub Check: Build With Lint
src/migrations/backfillFileOpeningDefaults.test.ts
[failure] 186-186:
Property 'fileOpening' does not exist on type '{ id: string; name: string; type: string; }'.
[failure] 180-180:
Property 'fileOpening' does not exist on type '{ id: string; name: string; type: string; }'.
[failure] 174-174:
Property 'fileOpening' does not exist on type '{ id: string; name: string; type: string; }'.
[failure] 168-168:
Property 'fileOpening' does not exist on type '{ id: string; name: string; type: string; }'.
[failure] 162-162:
Property 'fileOpening' does not exist on type '{ id: string; name: string; type: string; }'.
[failure] 150-150:
Property 'fileOpening' does not exist on type '{ id: string; name: string; type: string; }'.
[failure] 144-144:
Property 'fileOpening' does not exist on type '{ id: string; name: string; type: string; openFileInNewTab: { enabled: boolean; direction: string; focus: boolean; }; openFileInMode: string; }'.
src/migrations/backfillFileOpeningDefaults.ts
[failure] 54-54:
Argument of type 'Partial | undefined' is not assignable to parameter of type '(Partial & ({ location: OpenLocation; direction: "vertical" | "horizontal"; mode: FileViewMode2; focus: boolean; } | { ...; })) | null | undefined'.
🔇 Additional comments (18)
src/settings.ts (1)
50-50: LGTM! Migration flag properly added.The new
backfillFileOpeningDefaultsmigration flag follows the established pattern and is correctly initialized tofalsein the default settings.Also applies to: 91-91
src/engine/CaptureChoiceEngine.template-property-types.test.ts (1)
40-40: LGTM! Test mock updated correctly.The test mock settings correctly include the new migration flag, maintaining alignment with the production settings schema.
src/utils/fileOpeningDefaults.ts (2)
1-8: LGTM! Clean type definition.The
FileOpeningSettingstype is well-structured with appropriate imported types and clear property definitions.
10-15: LGTM! Sensible defaults established.The default file opening settings are reasonable: new files open in tabs with vertical splits in default mode and receive focus.
src/engine/TemplateChoiceEngine.notice.test.ts (1)
35-35: LGTM! Test mock synchronized with settings schema.The migration flag has been correctly added to the test mock settings.
src/engine/MacroChoiceEngine.notice.test.ts (1)
35-35: LGTM! Test mock properly updated.The new migration flag is correctly included in the mock settings, maintaining consistency with the broader settings schema changes.
src/engine/TemplateChoiceEngine.ts (1)
188-188: LGTM!Passing the normalized
fileOpeningobject ensures consistent defaults are applied when opening the file.src/types/choices/TemplateChoice.ts (1)
6-6: LGTM!Centralizing the file opening defaults through
normalizeFileOpening()improves maintainability and ensures consistency across choice types.Also applies to: 43-43
src/types/choices/CaptureChoice.ts (1)
6-6: LGTM!The normalization pattern is consistently applied across both Template and Capture choice types.
Also applies to: 79-79
src/engine/CaptureChoiceEngine.ts (1)
196-196: LGTM!Passing the normalized
fileOpeningobject ensures consistent defaults are applied.src/gui/ChoiceBuilder/choiceBuilder.ts (1)
6-9: LGTM! Well-structured centralization of file opening defaults.The changes properly delegate default initialization to the centralized
normalizeFileOpeningutility and adopt the sharedFileOpeningSettingstype, improving consistency across the codebase.Also applies to: 137-137, 139-139
src/engine/CaptureChoiceEngine.selection.test.ts (1)
174-210: Excellent test coverage for default file opening behavior.The test properly validates that when
fileOpeningis undefined, the engine normalizes it to the expected defaults before passing it toopenFile. The test setup and assertions are clear and comprehensive.src/engine/CaptureChoiceEngine.notice.test.ts (1)
35-35: LGTM! Proper test mock maintenance.Correctly adds the new migration flag to the test settings mock to keep it aligned with the actual settings structure.
src/migrations/migrate.ts (1)
11-11: LGTM! Migration properly registered.The new
backfillFileOpeningDefaultsmigration is correctly imported and added to the migrations map, following the established pattern.Also applies to: 23-23
src/migrations/helpers/file-opening-legacy.ts (1)
1-49: LGTM! Clean legacy mapping implementation.The helper functions correctly handle legacy file-opening settings conversion:
coerceLegacyOpenFileInNewTabsafely normalizes various legacy input formats (boolean, object, null/undefined)createFileOpeningFromLegacyproperly maps legacy fields to the newFileOpeningSettingsstructure with appropriate defaults- Type validation and fallback logic are sound
src/migrations/helpers/choice-traversal.ts (1)
16-90: Solid traversal implementation with proper cycle detection.The traversal utilities correctly handle:
- Cycle prevention via visited set
- Multi choices and nested structures
- Macro choices and their commands
- Conditional and NestedChoice commands with defensive type checks
- Legacy macro support with type assertions where necessary
The defensive checks at lines 53-56 and 64-69 appropriately handle edge cases in legacy data.
src/migrations/migrateFileOpeningSettings.ts (2)
28-32: Verify migration condition handles failed coercion.The condition at line 32 checks
legacyTabRawbut useslegacyTab(the coerced version). IfcoerceLegacyOpenFileInNewTabreturnsnull(e.g., whenlegacyTabRawis a string), the migration still proceeds with all default values via optional chaining at lines 35-37.Consider whether the condition should also check
legacyTabto skip migration when coercion fails:if (!templateOrCaptureChoice.fileOpening && legacyTab) {This ensures migration only occurs when legacy settings are successfully coerced.
Do you want the migration to apply defaults when legacy settings are invalid, or skip migration entirely?
7-11: Excellent refactor to use centralized helpers.The migration now correctly delegates to:
walkAllChoicesfor comprehensive traversal (including nested and legacy structures)coerceLegacyOpenFileInNewTabandcreateFileOpeningFromLegacyfor consistent legacy handlingThis eliminates code duplication and aligns with the shared migration utilities introduced in this PR.
Also applies to: 52-52
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/migrations/migrateFileOpeningSettings.ts (1)
28-48: Optional: Redundant optional chaining.Lines 35-37 use optional chaining (
legacyTab?.enabled, etc.) even though line 32's condition ensureslegacyTabis truthy. While this defensive pattern is safe and may protect against future refactoring, it's technically redundant.Consider simplifying if you prefer directness:
const tabSettings = { enabled: legacyTab.enabled ?? false, direction: legacyTab.direction ?? "vertical", focus: legacyTab.focus ?? true, };That said, the current code is perfectly safe and may be preferred for resilience.
src/utils/fileOpeningDefaults.ts (2)
30-46: Consider safer type construction pattern.Line 30's type assertion is aggressive—it asserts a potentially incomplete object as
FileOpeningSettings & TExtrasbefore the required fields are populated (lines 32-43). While this works in the current code, it could cause confusion during future modifications.Consider a safer pattern:
♻️ Suggested refactor for improved type safety
-const normalized = { ...(fileOpening ?? {}) } as FileOpeningSettings & TExtras; - -if (normalized.location == null) { - normalized.location = DEFAULT_FILE_OPENING.location; -} -if (normalized.direction == null) { - normalized.direction = DEFAULT_FILE_OPENING.direction; -} -if (normalized.mode == null) { - normalized.mode = DEFAULT_FILE_OPENING.mode; -} -if (normalized.focus == null) { - normalized.focus = DEFAULT_FILE_OPENING.focus; -} - -return normalized; +return { + location: fileOpening?.location ?? DEFAULT_FILE_OPENING.location, + direction: fileOpening?.direction ?? DEFAULT_FILE_OPENING.direction, + mode: fileOpening?.mode ?? DEFAULT_FILE_OPENING.mode, + focus: fileOpening?.focus ?? DEFAULT_FILE_OPENING.focus, + ...(fileOpening ?? {}), +} as FileOpeningSettings & TExtras;This builds the complete object in one expression, making the type assertion provably safe.
17-46: Optional: Consider runtime validation for enum-like fields.The function doesn't validate that
directionis "vertical" or "horizontal", or thatlocation/modematch their expected enum values. While TypeScript provides compile-time safety, runtime validation could catch issues from untyped sources (e.g., JSON parsing, external APIs).This is likely intentional to keep the function lightweight, but consider adding validation if robustness is a priority.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/migrations/backfillFileOpeningDefaults.test.tssrc/migrations/migrateFileOpeningSettings.tssrc/utils/fileOpeningDefaults.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/migrations/backfillFileOpeningDefaults.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,mts,mjs,js,json}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,mts,mjs,js,json}: Use tab indentation with width 2 in TypeScript and configuration files (enforced by Biome).
Follow an 80-character line guide (enforced by Biome).
Files:
src/migrations/migrateFileOpeningSettings.tssrc/utils/fileOpeningDefaults.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use camelCase for variables and functions in TypeScript.
Prefer type-only imports in TypeScript.
Files:
src/migrations/migrateFileOpeningSettings.tssrc/utils/fileOpeningDefaults.ts
**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components.
Files:
src/migrations/migrateFileOpeningSettings.tssrc/utils/fileOpeningDefaults.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Route logging through the
loggerutilities for consistent output.
Files:
src/migrations/migrateFileOpeningSettings.tssrc/utils/fileOpeningDefaults.ts
src/{engine,services,utils}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Structure production code so Obsidian dependencies are injected behind interfaces to enable unit testing without loading real Obsidian modules.
Files:
src/utils/fileOpeningDefaults.ts
🔇 Additional comments (3)
src/utils/fileOpeningDefaults.ts (2)
1-15: LGTM! Clean type definition and sensible defaults.The
FileOpeningSettingstype andDEFAULT_FILE_OPENINGconstant provide a clear, centralized contract for file-opening behavior. The defaults (tab location, vertical split, default mode, auto-focus) are reasonable choices.
23-28: Correct merge behavior for full override mode.When
fillMissingOnlyis false, the function correctly merges defaults first, then applies overrides. The spread operator sequence ensures provided values take precedence.src/migrations/migrateFileOpeningSettings.ts (1)
52-52: walkAllChoices correctly traverses all choice types and structures.The implementation properly handles:
- All root-level choices via
plugin.settings.choices- Nested Multi-choice children through recursive
walkChoicecalls- Macro-embedded choices and legacy macros via
walkCommands- Circular references prevention using a
visitedSetThe migration is safe to use.
|
🎉 This PR is included in version 2.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
Testing
Migration/Release Impact
backfillFileOpeningDefaultsto populate missing file opening settingsSummary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.