Skip to content

fix(prose): keep bare numbered replies visible - #1330

Open
scui73414-bot wants to merge 2 commits into
Agent-Field:devfrom
scui73414-bot:codex/fix-bare-numbered-reply
Open

scui73414-bot wants to merge 2 commits into
Agent-Field:devfrom
scui73414-bot:codex/fix-bare-numbered-reply

Conversation

@scui73414-bot

Copy link
Copy Markdown

Summary

  • render an empty ordered-list item using its pending marker
  • preserve assistant replies such as 32. and 1024.
  • add regression coverage for the zero-row rendering case

Fixes #1072.

Testing

  • make test-focus PKGS=./internal/tui2/prose RUN='^TestABareNumberedReplyStillDraws$'
  • make test-touched BASE=upstream/dev
  • make test-laws

All listed tests pass. make test-quick reached an unrelated failure in TestTheManualQuotesBothTasksOpeningHeadingsExactly; I reproduced that same failure on a clean upstream/dev worktree.

Prepared with OpenAI Codex assistance.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Table seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@santoshkumarradha

Copy link
Copy Markdown
Member

Hey, thanks for the PR. Could you sign the CLA when you get a chance so we can review it?

@AbirAbbas AbirAbbas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Checked this against dev: 32. and 1024. do render to "" on 9258f7052, the three lines in renderer.list fix it, and the new test fails on unpatched render.go — so it's a real regression test and not a tautology. I also ran the PR-gate steps locally on this head (build, vet, gofmt, packed corpora, changelog check, manual gates, make test-laws, make test PKGS=./internal/tui2/prose) and they're all green, and walked the empty-item shapes goldmark can produce (nested empty items, an empty item inside a blockquote, loose lists, a marker wider than the pane) without finding a row that's now wrong or over the width ceiling.

One gap in the coverage, inline.

// TestABareNumberedReplyStillDraws holds the reply boundary where Markdown
// parses a number and full stop as an ordered list with an empty item.
func TestABareNumberedReplyStillDraws(t *testing.T) {
for _, src := range []string{"32.", "1024."} {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The reply-shaped case isn't the only thing that was being dropped — any body-less item took its own marker down with it, anywhere in a list. On dev, "1. one\n2.\n3. three" renders as

1. one
3. three

and with this change it renders all three rows. That's the case a future refactor of the pop/pending contract is most likely to re-break, and it's silent when it does, so it's worth an assertion here alongside the two bare numbers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tui3: a reply that is only a number and a full stop draws nothing

4 participants