Add jira comments subcommand - #9
Conversation
Rewrite HTML tag stripper to preserve links (as "text (url)") and images (as "[image: url]") instead of stripping all tags. Add Atlassian Document Format (ADF) renderer for Jira Cloud descriptions, handling text with link marks, inlineCard/embedCard smart links, mentions, hard breaks, and media nodes. Fix formatJiraIssue to handle ADF object descriptions in addition to plain string descriptions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Jira command-line tool by introducing a dedicated subcommand for viewing issue comments. It improves the user experience by providing a human-readable text output for comments and descriptions, which includes proper rendering of rich text content from Jira's Atlassian Document Format. Additionally, it expands the JSON output for issue details to include comments, facilitating more comprehensive programmatic integrations. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds a jira comments subcommand and enhances the HTML and Jira ADF formatters. The changes are generally well-implemented. I've identified a few areas for improvement, including an issue with error handling that could lead to corrupted output, and some opportunities to improve code consistency and maintainability by using standard library functions and reducing magic strings and code duplication.
| result.append(allocator, c) catch {}; | ||
| // Other tag - add space after for readability | ||
| if (close + 1 < html.len and html[close + 1] != ' ' and html[close + 1] != '\n') { | ||
| result.append(allocator, ' ') catch {}; |
There was a problem hiding this comment.
The use of catch {} here and in several other places in this function (lines 109, 111, 113, 115, 117, 126) will silently ignore memory allocation errors when appending to the result ArrayList. This can lead to corrupted or truncated output without any indication of failure. Since stripHtmlTags is a failable function (returns ![]u8), it should propagate these errors by using try.
try result.append(allocator, ' ');
There was a problem hiding this comment.
Fixed in the base branch (1e12dd1, rebased into this PR). All catch {} calls in stripHtmlTags now use try to propagate allocation errors.
| while (p < tag_inner.len and (tag_inner[p] == ' ' or tag_inner[p] == '\t')) : (p += 1) {} | ||
| if (p >= tag_inner.len or tag_inner[p] != '=') continue; | ||
| p += 1; | ||
| while (p < tag_inner.len and (tag_inner[p] == ' ' or tag_inner[p] == '\t')) : (p += 1) {} |
There was a problem hiding this comment.
The while loops to skip whitespace only check for space and tab characters (' ' or '\t'). However, other parts of the code (e.g., line 16, line 58) treat newline and carriage return as whitespace as well. For consistency and to handle all whitespace characters, it's better to use std.ascii.isWhitespace.
while (p < tag_inner.len and std.ascii.isWhitespace(tag_inner[p])) : (p += 1) {}
if (p >= tag_inner.len or tag_inner[p] != '=') continue;
p += 1;
while (p < tag_inner.len and std.ascii.isWhitespace(tag_inner[p])) : (p += 1) {}
There was a problem hiding this comment.
Fixed in the base branch (1e12dd1). extractHtmlAttr now uses std.ascii.isWhitespace for all whitespace checks.
| if (marks_val == .array) { | ||
| for (marks_val.array.items) |mark| { | ||
| if (mark != .object) continue; | ||
| const mt = if (mark.object.get("type")) |t| if (t == .string) t.string else null else null; |
There was a problem hiding this comment.
This pattern of extracting a string from a JSON value is repeated multiple times in writeAdfText (e.g., lines 215, 243, 254). To improve readability and reduce code duplication, consider creating a helper function.
For example:
fn getJsonString(obj: std.json.ObjectMap, key: []const u8) ?[]const u8 {
const value = obj.get(key) orelse return null;
if (value.* == .string) {
return value.*.string;
}
return null;
}Then you could simplify this line to:
if (getJsonString(mark.object, "type")) |mark_type| { ... }
There was a problem hiding this comment.
Acknowledged, but skipping this one. A helper would add an indirection layer without significant readability gain given how Zig handles optional chaining. The pattern is consistent across the codebase.
| return; | ||
| } | ||
| const response = try jira.getIssue(args[1], null); | ||
| const issue_fields: ?[]const u8 = if (output_format == .json) "summary,description,status,assignee,reporter,labels,priority,created,updated,issuetype,comment" else null; |
There was a problem hiding this comment.
The long string of issue fields is a "magic string". It's better to define it as a constant at the top of handleJiraCommand or at the file level. This improves readability and makes it easier to manage if it needs to be changed or reused.
For example:
const JSON_ISSUE_FIELDS = "summary,description,status,assignee,reporter,labels,priority,created,updated,issuetype,comment";
// ...
const issue_fields: ?[]const u8 = if (output_format == .json) JSON_ISSUE_FIELDS else null;There was a problem hiding this comment.
Fixed in bd61129. Extracted to a named constant json_issue_fields.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35d00854ab
ℹ️ 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".
| stack[depth] = .{ .items = content_val.array.items, .idx = 0 }; | ||
| depth += 1; | ||
| continue; // process children before emitting newline |
There was a problem hiding this comment.
Preserve block newlines when rendering ADF content
When a node has a content array, this continue skips the block-level newline logic below, so paragraph/list/heading boundaries are never emitted for the common case where those nodes contain children. In practice, multi-paragraph Jira descriptions and comment bodies rendered via writeAdfText get concatenated into a single run of text (e.g., paragraph 1 immediately followed by paragraph 2), which makes the new jira comments text output and ADF-backed issue descriptions hard to read.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the base branch (1e12dd1, rebased into this PR). The DFS stack now tracks is_block per frame and emits newlines when block-node frames are exhausted, so paragraphs and headings in comments are properly separated.
Fix off-by-one in extractHtmlAttr loop bound and use std.ascii.isWhitespace for consistent whitespace checks. Propagate allocation errors in stripHtmlTags instead of silently swallowing them. Fix writeAdfText to emit newlines after block-level nodes by tracking is_block in the DFS stack, so paragraphs and headings are properly separated. Add [image] fallback for external media nodes missing a URL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add `jira comments <issue-key>` command to display issue comments with formatted ADF body text. Text output shows timestamp, author, and comment body. JSON output passes through the raw API response. Also include the comment field in `jira issue --format=json` output for programmatic use.
Extract JSON issue fields string to a named constant for readability.
35d0085 to
bd61129
Compare
Summary
jira comments <issue-key> [--format=text|json]command to display issue commentscommentfield injira issue --format=jsonfor programmatic access to comments inlineDepends on #7.
Test plan
jira comments <key>displays comments with correct date, author, and bodyjira comments <key> --format=jsonreturns raw API responsejira issue <key> --format=jsonnow includes comments in the responsejira issue <key>text output is unchangedjira helpand global help list the new command🤖 Generated with Claude Code