fix: treat code mark as exclusive in ADF inline formatting#27
Conversation
Jira rejects ADF documents where the code mark is combined with other marks (strong, em, strike) on the same text node, returning 400 INVALID_INPUT. This happened when using inline code inside bold, italic, or strikethrough (e.g. **bold `code` text**). The code mark is exclusive in the ADF spec and must not overlap with other marks. When nesting inline code inside formatted text, the outer mark is now skipped for code nodes.
Summary of ChangesHello @Hinne1, 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 addresses a critical compatibility issue with Jira's ADF document processing. Previously, combining inline code with other formatting like bold or italic would lead to document rejection. The changes ensure that the 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
|
Merging this branch will not change overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves the issue with Jira's exclusive code mark handling in ADF. The introduction of the hasCodeMark and addMarkToContent helper functions is a clean solution that also refactors parseInline to be more modular and less repetitive. The new tests in markdown_code_mark_test.go are a great addition to validate the fix. I have a few suggestions to make these new tests even more robust by adding more specific assertions.
| func TestMarkdownToADF_CodeInsideItalic(t *testing.T) { | ||
| adf := MarkdownToADF("*italic `code` here*") | ||
|
|
||
| para := adf.Content[0] | ||
| for _, c := range para.Content { | ||
| if hasCodeMark(c) { | ||
| if len(c.Marks) != 1 { | ||
| t.Errorf("code node should have only code mark, got %v", c.Marks) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This test correctly verifies that code marks are exclusive. To make it more robust, I suggest expanding it to also assert that the non-code parts of the string correctly receive the em mark. This would provide a more complete validation, similar to TestMarkdownToADF_CodeInsideBold.
func TestMarkdownToADF_CodeInsideItalic(t *testing.T) {
adf := MarkdownToADF("*italic `code` here*")
if len(adf.Content) != 1 {
t.Fatalf("expected 1 block, got %d", len(adf.Content))
}
para := adf.Content[0]
if para.Type != "paragraph" {
t.Fatalf("expected paragraph, got %s", para.Type)
}
if len(para.Content) != 3 {
b, _ := json.MarshalIndent(adf, "", " ")
t.Fatalf("expected 3 inline nodes, got %d:\n%s", len(para.Content), b)
}
// "italic " should have em mark
if para.Content[0].Text != "italic " {
t.Errorf("expected 'italic ', got %q", para.Content[0].Text)
}
if len(para.Content[0].Marks) != 1 || para.Content[0].Marks[0].Type != "em" {
t.Errorf("expected [em] marks, got %v", para.Content[0].Marks)
}
// "code" should have ONLY code mark
if para.Content[1].Text != "code" {
t.Errorf("expected 'code', got %q", para.Content[1].Text)
}
if len(para.Content[1].Marks) != 1 || para.Content[1].Marks[0].Type != "code" {
t.Errorf("expected [code] marks only, got %v", para.Content[1].Marks)
}
// " here" should have em mark
if para.Content[2].Text != " here" {
t.Errorf("expected ' here', got %q", para.Content[2].Text)
}
if len(para.Content[2].Marks) != 1 || para.Content[2].Marks[0].Type != "em" {
t.Errorf("expected [em] marks, got %v", para.Content[2].Marks)
}
}| func TestMarkdownToADF_CodeInsideStrikethrough(t *testing.T) { | ||
| adf := MarkdownToADF("~~deleted `code` here~~") | ||
|
|
||
| para := adf.Content[0] | ||
| for _, c := range para.Content { | ||
| if hasCodeMark(c) { | ||
| if len(c.Marks) != 1 { | ||
| t.Errorf("code node should have only code mark, got %v", c.Marks) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Similar to the italic test, this test for strikethrough can be strengthened. Explicitly checking that the text segments around the code block have the strike mark would make the test more comprehensive and help prevent future regressions.
func TestMarkdownToADF_CodeInsideStrikethrough(t *testing.T) {
adf := MarkdownToADF("~~deleted `code` here~~")
if len(adf.Content) != 1 {
t.Fatalf("expected 1 block, got %d", len(adf.Content))
}
para := adf.Content[0]
if para.Type != "paragraph" {
t.Fatalf("expected paragraph, got %s", para.Type)
}
if len(para.Content) != 3 {
b, _ := json.MarshalIndent(adf, "", " ")
t.Fatalf("expected 3 inline nodes, got %d:\n%s", len(para.Content), b)
}
// "deleted " should have strike mark
if para.Content[0].Text != "deleted " {
t.Errorf("expected 'deleted ', got %q", para.Content[0].Text)
}
if len(para.Content[0].Marks) != 1 || para.Content[0].Marks[0].Type != "strike" {
t.Errorf("expected [strike] marks, got %v", para.Content[0].Marks)
}
// "code" should have ONLY code mark
if para.Content[1].Text != "code" {
t.Errorf("expected 'code', got %q", para.Content[1].Text)
}
if len(para.Content[1].Marks) != 1 || para.Content[1].Marks[0].Type != "code" {
t.Errorf("expected [code] marks only, got %v", para.Content[1].Marks)
}
// " here" should have strike mark
if para.Content[2].Text != " here" {
t.Errorf("expected ' here', got %q", para.Content[2].Text)
}
if len(para.Content[2].Marks) != 1 || para.Content[2].Marks[0].Type != "strike" {
t.Errorf("expected [strike] marks, got %v", para.Content[2].Marks)
}
}| func TestMarkdownToADF_BoldWithCodeAndCodeBlock(t *testing.T) { | ||
| // This was the exact combination that caused INVALID_INPUT from Jira | ||
| md := "**Migration: Falsche `s_action` korrigieren**\n\n```sql\nSELECT 1\n```" | ||
| adf := MarkdownToADF(md) | ||
|
|
||
| if len(adf.Content) != 2 { | ||
| b, _ := json.MarshalIndent(adf, "", " ") | ||
| t.Fatalf("expected 2 blocks (paragraph + codeBlock), got %d:\n%s", len(adf.Content), b) | ||
| } | ||
|
|
||
| // Verify no code node has additional marks | ||
| para := adf.Content[0] | ||
| for _, c := range para.Content { | ||
| if hasCodeMark(c) && len(c.Marks) > 1 { | ||
| t.Errorf("code node should have only code mark, got %v", c.Marks) | ||
| } | ||
| } | ||
|
|
||
| // Verify code block is present | ||
| if adf.Content[1].Type != "codeBlock" { | ||
| t.Errorf("expected codeBlock, got %s", adf.Content[1].Type) | ||
| } | ||
| } |
There was a problem hiding this comment.
This test covers the specific case that triggered the bug, which is excellent. To make it even more solid, I suggest adding assertions to verify that the non-code parts of the bolded string also have the strong mark. This provides a more complete verification of the paragraph's content and protects against regressions.
func TestMarkdownToADF_BoldWithCodeAndCodeBlock(t *testing.T) {
// This was the exact combination that caused INVALID_INPUT from Jira
md := "**Migration: Falsche `s_action` korrigieren**\n\n```sql\nSELECT 1\n```"
adf := MarkdownToADF(md)
if len(adf.Content) != 2 {
b, _ := json.MarshalIndent(adf, "", " ")
t.Fatalf("expected 2 blocks (paragraph + codeBlock), got %d:\n%s", len(adf.Content), b)
}
// Verify paragraph content
para := adf.Content[0]
if para.Type != "paragraph" {
t.Fatalf("expected paragraph, got %s", para.Type)
}
if len(para.Content) != 3 {
b, _ := json.MarshalIndent(para, "", " ")
t.Fatalf("expected 3 inline nodes in paragraph, got %d:\n%s", len(para.Content), b)
}
// "Migration: Falsche " should have strong mark
if para.Content[0].Text != "Migration: Falsche " {
t.Errorf("expected 'Migration: Falsche ', got %q", para.Content[0].Text)
}
if len(para.Content[0].Marks) != 1 || para.Content[0].Marks[0].Type != "strong" {
t.Errorf("expected [strong] marks, got %v", para.Content[0].Marks)
}
// "s_action" should have ONLY code mark
if para.Content[1].Text != "s_action" {
t.Errorf("expected 's_action', got %q", para.Content[1].Text)
}
if len(para.Content[1].Marks) != 1 || para.Content[1].Marks[0].Type != "code" {
t.Errorf("expected [code] marks only, got %v", para.Content[1].Marks)
}
// " korrigieren" should have strong mark
if para.Content[2].Text != " korrigieren" {
t.Errorf("expected ' korrigieren', got %q", para.Content[2].Text)
}
if len(para.Content[2].Marks) != 1 || para.Content[2].Marks[0].Type != "strong" {
t.Errorf("expected [strong] marks, got %v", para.Content[2].Marks)
}
// Verify code block is present
if adf.Content[1].Type != "codeBlock" {
t.Errorf("expected codeBlock, got %s", adf.Content[1].Type)
}
}1 participant
Summary
codemark is combined with other marks (strong,em,strike) on the same text node, returning400 INVALID_INPUT**boldcodetext**)codemark is exclusive in the ADF spec and must not overlap with other marksaddMarkToContent()helper that skips adding outer marks to code nodesTest plan
markdown_code_mark_test.gowith tests for code inside bold, italic, strikethrough, and the combo that originally triggered the bugatl issue comment addthat was returning 400 now works