Fix blank-line-after-list suppressing paragraph break#363
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f89367a02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR addresses a regression in the Markdown-to-HTML renderer where a blank line after a list no longer produced the expected <br> separation before the following paragraph.
Changes:
- Set
pendingBreak = truewhen consuming blank lines while parsing inside a list. - Add a regression test covering list → blank line → paragraph rendering.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/richtext/richtext.go | Adjusts blank-line handling in list parsing to preserve paragraph separation after lists. |
| internal/richtext/richtext_test.go | Adds a test case for list followed by a blank line and then a paragraph. |
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
04c9962 to
1af6f65
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The recent list continuation fix (6ed164f) swallowed blank lines inside lists to prevent them from splitting the list, but failed to set pendingBreak. This meant a blank line between a list and the following paragraph no longer produced the <br> separator. Set pendingBreak when eating blank lines inside lists so content after the list gets proper separation.
Blank lines between list items set pendingBreak, but if the list continues (another item or a continuation line), the break should not leak to whatever follows the list. Clear pendingBreak when processing list items and continuation lines so only the last blank before exiting the list produces a separator.
1af6f65 to
b18c144
Compare
Cherry-picked fixes from pending main PRs: - Fix --assignee filter ignored with --list (#369) - Fix chat post JSON output contract and upload error handling (#365) - Fix schedule update silently dropping --attach without --description (#364) - Fix blank-line-after-list suppressing paragraph break (#363) - Fix campfire messages --limit being ignored (#367)
…accounts (#373) * Bump basecamp-sdk to v0.7.1 Fixes Person.Id deserialization (normalizeJSON pre-unmarshal for notification responses) and Account.Logo type (string → AccountLogo object). Adapts two breaking changes: Cards().Move() and Todos().Reposition() now take optional params. Adds AVIF/HEIC to richtext MIME extension map for logo upload support. * Merge upstream fixes (#363, #364, #365, #367, #369) Also fixes: chat upload error now wraps converted SDK error instead of raw error; schedule update --attach without --description now fetches current description to append rather than replace. * Add gauges command for project progress tracking Subcommands: list (account-wide), needles/needle (project-scoped), create/update/delete, enable/disable. Create validates --position (0-100), --color, --notify (requires --subscriptions for custom mode). * Add assignments command for viewing my assignments Subcommands: list, completed, due (with scope filter). Shortcut pattern — bare invocation shows priorities + non-priorities. * Add notifications command, extend accounts, register all new commands Notifications: list (paginated with dynamic next-page breadcrumb) and read (resolves IDs to SGIDs from --page). Accounts: show, update, logo upload (5MB + image type validation) and remove. Registers all new commands in root.go, catalog, and test harness. * Update docs and surface for v0.7.1 commands API-COVERAGE.md: 42 sections, 155 endpoints, SDK v0.7.1. SKILL.md: gauges, assignments, notifications, accounts sections. .surface: 148 commands, 422 flags.
Summary
pendingBreak, so a blank line between a list and the following paragraph no longer produced the<br>separatorpendingBreak = truewhen eating blank lines inside lists — safe because list item processing never consumespendingBreak, so it only fires when content follows the listTest plan
go test ./internal/richtext/— all existing tests pass, new test case validates the fixmake lint— cleanSummary by cubic
Fixes a regression where a blank line after a list didn’t insert the
<br>before the next paragraph. SetspendingBreakon blank lines inside lists and clears it when the list continues (new item or continuation), so only the last blank before exiting the list adds the separator; tests cover list → blank → paragraph and ensure blanks between items don’t leak a break.Written for commit b18c144. Summary will update on new commits.