-
-
Notifications
You must be signed in to change notification settings - Fork 169
fix: preserve blank lines after insert after match #1054
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe changes implement functionality to properly handle inserting content after lines followed by blank lines. When using "Insert After", the formatter now scans forward from the matched line to position the cursor after any trailing blank/whitespace lines, preserving spacing patterns. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/docs/Choices/CaptureChoice.md (1)
60-70: LGTM: Documentation clearly explains the feature.The explanation and example accurately demonstrate the blank-line-preservation behavior.
💡 Optional: Consider showing before/after states
The example could be slightly clearer by explicitly showing the before and after states:
Example (Insert After `# H` with content `X`): +Before: +```markdown +# H + +A +``` + +After: ```markdown # H X AThis makes it immediately clear where X was inserted. </details> </blockquote></details> <details> <summary>src/formatters/captureChoiceFormatter-frontmatter.test.ts (1)</summary><blockquote> `181-309`: **LGTM: Comprehensive regression test suite.** The tests thoroughly verify the blank-line-preservation behavior across multiple scenarios, including edge cases like CRLF content and EOF matches. The helper functions effectively reduce duplication. Based on learnings, this provides excellent regression coverage for the bug fix. <details> <summary>💡 Optional: Consider adding a test for trailing blank lines at EOF</summary> The current tests cover most edge cases, but you could add a test for when the matched line is followed by blank lines that extend to EOF: ```typescript it('handles blank lines at EOF after the match', async () => { const { formatter, file } = createFormatter(); const choice = createInsertAfterChoice('# H'); const fileContent = '# H\n\n\n'; const result = await formatter.formatContentWithFile( 'X\n', choice, fileContent, file, ); expect(result).toBe('# H\n\n\nX\n'); });This would verify that the
scanLimitlogic correctly handles trailing blank lines.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/docs/Choices/CaptureChoice.md(1 hunks)src/formatters/captureChoiceFormatter-frontmatter.test.ts(1 hunks)src/formatters/captureChoiceFormatter.ts(2 hunks)src/gui/ChoiceBuilder/captureChoiceBuilder.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/gui/ChoiceBuilder/captureChoiceBuilder.tssrc/formatters/captureChoiceFormatter-frontmatter.test.tssrc/formatters/captureChoiceFormatter.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/gui/ChoiceBuilder/captureChoiceBuilder.tssrc/formatters/captureChoiceFormatter-frontmatter.test.tssrc/formatters/captureChoiceFormatter.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/gui/ChoiceBuilder/captureChoiceBuilder.tssrc/formatters/captureChoiceFormatter-frontmatter.test.tssrc/formatters/captureChoiceFormatter.ts
docs/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Keep user-facing docs in
docs/directory
Files:
docs/docs/Choices/CaptureChoice.md
🧠 Learnings (1)
📚 Learning: 2025-12-09T21:20:52.425Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.425Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes
Applied to files:
src/formatters/captureChoiceFormatter-frontmatter.test.ts
🧬 Code graph analysis (1)
src/formatters/captureChoiceFormatter-frontmatter.test.ts (3)
src/formatters/captureChoiceFormatter.ts (1)
CaptureChoiceFormatter(16-399)src/types/choices/ICaptureChoice.ts (1)
ICaptureChoice(5-46)src/services/choiceService.ts (1)
createChoice(30-34)
🔇 Additional comments (3)
src/gui/ChoiceBuilder/captureChoiceBuilder.ts (1)
410-413: LGTM: Clear user-facing guidance.The updated description accurately explains the blank-line-preservation behavior and provides helpful context about using headings as targets.
src/formatters/captureChoiceFormatter.ts (2)
201-224: LGTM: Well-implemented blank-line-scanning logic.The helper correctly:
- Scans forward from the matched line to find contiguous blank lines
- Handles EOF edge cases by excluding the trailing empty string from
split("\n")when the body ends with a newline- Preserves existing behavior when no blank lines follow
259-265: LGTM: Correct integration of blank-line handling.The helper is correctly called only when
!insertAtEnd, preserving the existing section-end logic wheninsertAtEndis enabled.
Summary
Testing