Skip to content

Fix blank-line-after-list suppressing paragraph break#363

Merged
jeremy merged 2 commits intomainfrom
fix-richtext-list-break-regression
Mar 24, 2026
Merged

Fix blank-line-after-list suppressing paragraph break#363
jeremy merged 2 commits intomainfrom
fix-richtext-list-break-regression

Conversation

@jeremy
Copy link
Copy Markdown
Member

@jeremy jeremy commented Mar 24, 2026

Summary

  • The list continuation fix (6ed164f) swallowed blank lines inside lists without setting pendingBreak, so a blank line between a list and the following paragraph no longer produced the <br> separator
  • Sets pendingBreak = true when eating blank lines inside lists — safe because list item processing never consumes pendingBreak, so it only fires when content follows the list
  • Adds test case for list → blank line → paragraph

Test plan

  • go test ./internal/richtext/ — all existing tests pass, new test case validates the fix
  • make lint — clean

Summary by cubic

Fixes a regression where a blank line after a list didn’t insert the <br> before the next paragraph. Sets pendingBreak on 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.

Copilot AI review requested due to automatic review settings March 24, 2026 16:41
@github-actions github-actions bot added tests Tests (unit and e2e) bug Something isn't working labels Mar 24, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread internal/richtext/richtext.go
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 = true when 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.

Comment thread internal/richtext/richtext.go
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Copilot AI review requested due to automatic review settings March 24, 2026 17:02
@jeremy jeremy force-pushed the fix-richtext-list-break-regression branch from 04c9962 to 1af6f65 Compare March 24, 2026 17:02
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@jeremy jeremy enabled auto-merge (squash) March 24, 2026 19:06
jeremy added 2 commits March 24, 2026 12:18
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.
@jeremy jeremy force-pushed the fix-richtext-list-break-regression branch from 1af6f65 to b18c144 Compare March 24, 2026 19:18
@jeremy jeremy merged commit 8ac3e9d into main Mar 24, 2026
26 checks passed
@jeremy jeremy deleted the fix-richtext-list-break-regression branch March 24, 2026 19:21
jeremy added a commit that referenced this pull request Mar 25, 2026
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)
jeremy added a commit that referenced this pull request Mar 25, 2026
jeremy added a commit that referenced this pull request Mar 25, 2026
jeremy added a commit that referenced this pull request Mar 25, 2026
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.
jeremy added a commit that referenced this pull request Mar 25, 2026
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.
jeremy added a commit that referenced this pull request Mar 25, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working tests Tests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants