Skip to content

fix(jtk): interpret escape sequences in comment body#188

Merged
rianjs merged 1 commit intomainfrom
fix/167-comment-newlines
Mar 28, 2026
Merged

fix(jtk): interpret escape sequences in comment body#188
rianjs merged 1 commit intomainfrom
fix/167-comment-newlines

Conversation

@rianjs
Copy link
Copy Markdown
Contributor

@rianjs rianjs commented Mar 28, 2026

Summary

  • Apply text.InterpretEscapes() to comment body before ADF conversion
  • Matches existing behavior for issue descriptions in issues create and issues update
  • One-line change

Closes #167

Example

# Before: rendered as literal \n
# After: renders as two lines
jtk comments add PROJ-123 --body "Line one\nLine two"

Test plan

  • make build && make test passes
  • Integration test: add comment with \n and verify line breaks in Jira

Apply InterpretEscapes to comment body text so that \n, \t, and other
C-style escape sequences are processed before ADF conversion. This
matches the existing behavior for issue descriptions.

Closes #167
Copy link
Copy Markdown

@monit-reviewer monit-reviewer left a comment

Choose a reason for hiding this comment

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

Automated PR Review

Reviewed commit: 05907ae

Approved with 1 non-blocking suggestion below. Address at your discretion.

Summary

Reviewer Findings
harness-engineering:architecture-reviewer 1
harness-engineering:architecture-reviewer (1 findings)

💡 Suggestion - tools/jtk/internal/cmd/comments/comments.go:142

No unit test added for escape sequence interpretation in runAdd. The text.InterpretEscapes() function itself is presumably tested, but a targeted test confirming the wiring in runAdd (e.g., that \n in --body produces a newline in the API call) would make the regression surface explicit.


Completed in 3m 21s | $0.61
Field Value
Reviewers hybrid-synthesis, database:sql-reviewer, security:code-auditor, harness-engineering:knowledge-reviewer, harness-engineering:enforcement-reviewer, harness-engineering:architecture-reviewer, harness-engineering:legibility-reviewer
Reviewed by pr-review-daemon · monit-pr-reviewer
Duration 3m 21s (Reviewers: 41s · Synthesis: 55s)
Cost $0.61
Tokens 398.6k in / 10.9k out
Turns 52

}

comment, err := client.AddComment(ctx, issueKey, body)
comment, err := client.AddComment(ctx, issueKey, text.InterpretEscapes(body))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Low (harness-engineering:architecture-reviewer): No unit test added for escape sequence interpretation in runAdd. The text.InterpretEscapes() function itself is presumably tested, but a targeted test confirming the wiring in runAdd (e.g., that \n in --body produces a newline in the API call) would make the regression surface explicit.

@rianjs rianjs merged commit d2b6fe2 into main Mar 28, 2026
7 checks passed
@rianjs rianjs deleted the fix/167-comment-newlines branch March 28, 2026 18:31
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.

jtk comments add: newlines in --body rendered as literal \\n

2 participants