From 3414bce950cef3d40ef2c85283ef7c603511b8c9 Mon Sep 17 00:00:00 2001 From: Reto Date: Sun, 12 Jul 2026 14:34:27 +0200 Subject: [PATCH 1/2] fix: Keep the markdown pipeline ASCII in plain view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #7 made the issue view's own decorations ASCII in plain mode, but plain output isn't printed from Issue.String() — it goes through glamour, and descriptions and comments from Jira cloud go through the ADF translator first. Both layers add non-ASCII of their own, so `LC_ALL=C jira issue view --plain` still emitted characters the locale can't represent. Three sources, all decorations the tool adds around issue data: - glamour's `notty` style prefixes list items with a bullet and suffixes image alt text with an arrow. Its `ascii` style is a byte-for-byte copy of `notty`, so switching styles doesn't help; plain view now builds the style itself and overrides those two. - the ADF translator marks inline cards with a pin emoji. - the ADF translator replaces `<` and `>` with lookalike glyphs, because the markdown renderer would otherwise eat them as an HTML tag. In ASCII mode we backslash-escape instead, which renders as the original character — so `--plain` output can now be grepped for ``. Code blocks and inline code are left alone, since the renderer prints those verbatim and an escape would show up as a literal backslash. Non-plain output is unchanged: the ADF translator only drops to ASCII when the view asks it to, which it does when Display.Plain is set. What's left in plain output is the issue text itself — if someone typed a curly apostrophe into a description, it still prints. Transliterating user content is a separate decision from not decorating it. Addresses ankitpokhrel/jira-cli#213. --- internal/view/helper.go | 17 +++++++ internal/view/issue.go | 17 ++++--- internal/view/issue_test.go | 89 +++++++++++++++++++++++++++++++++++-- pkg/adf/adf.go | 15 ++++++- pkg/adf/adf_test.go | 51 +++++++++++++++++++++ pkg/adf/markdown.go | 45 ++++++++++++++++++- 6 files changed, 222 insertions(+), 12 deletions(-) diff --git a/internal/view/helper.go b/internal/view/helper.go index 68254fe2..0abeb72d 100644 --- a/internal/view/helper.go +++ b/internal/view/helper.go @@ -13,6 +13,7 @@ import ( "github.com/atotto/clipboard" "github.com/charmbracelet/glamour" + "github.com/charmbracelet/glamour/styles" "github.com/fatih/color" "github.com/mgutz/ansi" "github.com/rivo/tview" @@ -98,6 +99,22 @@ func MDRenderer() (*glamour.TermRenderer, error) { ) } +// plainMDRenderer constructs a markdown renderer for plain output. +// +// Glamour's "ascii" and "notty" styles are not ASCII: both decorate list items +// with a bullet and images with an arrow. Plain output must survive a non-UTF-8 +// locale, so we swap those two for ASCII equivalents. +func plainMDRenderer() (*glamour.TermRenderer, error) { + style := styles.ASCIIStyleConfig + style.Item.BlockPrefix = "* " + style.ImageText.Format = "Image: {{.text}}" + + return glamour.NewTermRenderer( + glamour.WithStyles(style), + glamour.WithWordWrap(wordWrap), + ) +} + func formatDateTime(dt, format, tz string) string { t, err := time.Parse(format, dt) if err != nil { diff --git a/internal/view/issue.go b/internal/view/issue.go index 9fbdc2ca..7ab7806e 100644 --- a/internal/view/issue.go +++ b/internal/view/issue.go @@ -179,6 +179,14 @@ func (i Issue) fragments() []fragment { return append(scraps, newBlankFragment(1), fragment{Body: i.footer()}, newBlankFragment(2)) } +// markdownTranslator translates ADF content, e.g. a description or a comment. +func (i Issue) markdownTranslator() *adf.MarkdownTranslator { + if i.Display.Plain { + return adf.NewMarkdownTranslator(adf.WithMarkdownASCII()) + } + return adf.NewMarkdownTranslator() +} + // fieldSeparator divides inline fields, e.g. the priority and status of a subtask. func (i Issue) fieldSeparator() string { if i.Display.Plain { @@ -287,7 +295,7 @@ func (i Issue) description() string { var desc string if adfNode, ok := i.Data.Fields.Description.(*adf.ADF); ok { - desc = adf.NewTranslator(adfNode, adf.NewMarkdownTranslator()).Translate() + desc = adf.NewTranslator(adfNode, i.markdownTranslator()).Translate() } else { desc = i.Data.Fields.Description.(string) desc = md.FromJiraMD(desc) @@ -438,7 +446,7 @@ func (i Issue) comments() []issueComment { c := i.Data.Fields.Comment.Comments[idx] var body string if adfNode, ok := c.Body.(*adf.ADF); ok { - body = adf.NewTranslator(adfNode, adf.NewMarkdownTranslator()).Translate() + body = adf.NewTranslator(adfNode, i.markdownTranslator()).Translate() } else { body = c.Body.(string) body = md.FromJiraMD(body) @@ -488,10 +496,7 @@ func (i Issue) footer() string { // renderPlain renders the issue in plain view. func (i Issue) renderPlain(w io.Writer) error { - r, err := glamour.NewTermRenderer( - glamour.WithStandardStyle("notty"), - glamour.WithWordWrap(wordWrap), - ) + r, err := plainMDRenderer() if err != nil { return err } diff --git a/internal/view/issue_test.go b/internal/view/issue_test.go index 767e8411..255fc23f 100644 --- a/internal/view/issue_test.go +++ b/internal/view/issue_test.go @@ -350,19 +350,100 @@ func decoratedIssue() *jira.Issue { } } -// Plain output must survive a non-UTF-8 locale like LC_ALL=C, so none of the -// decorations the view adds around issue data may be non-ASCII. +// adfBody is a body Jira cloud would return: a list, an inline card, a code +// block and angle brackets, each of which the markdown pipeline decorates. +func adfBody(text string) *adf.ADF { + return &adf.ADF{ + Version: 1, + DocType: "doc", + Content: []*adf.Node{ + { + NodeType: "paragraph", + Content: []*adf.Node{ + {NodeType: "text", NodeValue: adf.NodeValue{Text: text + " for & co"}}, + {NodeType: "inlineCard", Attributes: map[string]any{"url": "https://test.local/browse/TEST-9"}}, + }, + }, + { + NodeType: "bulletList", + Content: []*adf.Node{ + { + NodeType: "listItem", + Content: []*adf.Node{ + { + NodeType: "paragraph", + Content: []*adf.Node{ + {NodeType: "text", NodeValue: adf.NodeValue{Text: "list item"}}, + }, + }, + }, + }, + }, + }, + { + NodeType: "codeBlock", + Content: []*adf.Node{ + {NodeType: "text", NodeValue: adf.NodeValue{Text: "if (ac) {}"}}, + }, + }, + }, + } +} + +// Plain output must survive a non-UTF-8 locale like LC_ALL=C, so nothing the +// view and its markdown renderer add around issue data may be non-ASCII. func TestPlainIssueViewIsASCIIOnly(t *testing.T) { t.Parallel() + for _, tc := range []struct { + name string + body any + }{ + {name: "jira markdown body", body: "Test description\n\n* list item\n* another item"}, + {name: "adf body", body: adfBody("Test description")}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + data := decoratedIssue() + data.Fields.Description = tc.body + data.Fields.Comment.Comments[0].Body = tc.body + + issue := Issue{ + Server: "https://test.local", + Data: data, + Display: DisplayFormat{Plain: true}, + Options: IssueOption{NumComments: 2}, + } + + var b bytes.Buffer + assert.NoError(t, issue.renderPlain(&b)) + assert.Empty(t, nonASCII(b.String())) + }) + } +} + +// The ASCII escaping must not cost us any content: what the lookalike glyphs +// used to stand in for has to come out of the renderer as the real character. +func TestPlainIssueViewKeepsAngleBrackets(t *testing.T) { + t.Parallel() + + data := decoratedIssue() + data.Fields.Description = adfBody("Test description") + issue := Issue{ Server: "https://test.local", - Data: decoratedIssue(), + Data: data, Display: DisplayFormat{Plain: true}, Options: IssueOption{NumComments: 2}, } - assert.Empty(t, nonASCII(issue.String())) + var b bytes.Buffer + assert.NoError(t, issue.renderPlain(&b)) + + out := b.String() + assert.Contains(t, out, "") + assert.Contains(t, out, "if (ac) {}") } func TestIssueViewKeepsDecorationsWhenNotPlain(t *testing.T) { diff --git a/pkg/adf/adf.go b/pkg/adf/adf.go index 78bd1ca0..f39cc2a6 100644 --- a/pkg/adf/adf.go +++ b/pkg/adf/adf.go @@ -52,6 +52,12 @@ type TagCloser interface { Close(Connector) string } +// TextSanitizer escapes the text of a node for the target format. A +// TagOpenerCloser that doesn't implement it gets the default escaping. +type TextSanitizer interface { + Sanitize(string) string +} + // TagOpenerCloser wraps tag opener and closer. type TagOpenerCloser interface { TagOpener @@ -219,7 +225,7 @@ func (a *Translator) visit(n *Node, depth int) { } } - tag.WriteString(sanitize(n.Text)) + tag.WriteString(a.sanitize(n.Text)) // Close tags in reverse order. for _, m := range slices.Backward(opened) { @@ -232,6 +238,13 @@ func (a *Translator) visit(n *Node, depth int) { a.buf.WriteString(a.tsl.Close(n)) } +func (a *Translator) sanitize(s string) string { + if ts, ok := a.tsl.(TextSanitizer); ok { + return ts.Sanitize(s) + } + return sanitize(s) +} + func sanitize(s string) string { s = strings.TrimSpace(s) s = strings.TrimRight(s, "\n") diff --git a/pkg/adf/adf_test.go b/pkg/adf/adf_test.go index e0565cf6..540e15a3 100644 --- a/pkg/adf/adf_test.go +++ b/pkg/adf/adf_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "os" "testing" + "unicode" "github.com/stretchr/testify/assert" ) @@ -22,6 +23,56 @@ func TestADF(t *testing.T) { assert.Equal(t, expected, tr.Translate()) } +func TestADFMarkdownASCII(t *testing.T) { + data, err := os.ReadFile("./testdata/md.json") + assert.NoError(t, err) + + var adf ADF + err = json.Unmarshal(data, &adf) + assert.NoError(t, err) + + out := NewTranslator(&adf, NewMarkdownTranslator(WithMarkdownASCII())).Translate() + + for _, r := range out { + assert.LessOrEqual(t, r, rune(unicode.MaxASCII), "translated markdown must stay ASCII") + } + assert.Contains(t, out, "Inline Node https://antiklabs.atlassian.net") +} + +func TestADFMarkdownASCIIEscapesAngleBrackets(t *testing.T) { + t.Parallel() + + doc := ADF{ + Version: 1, + DocType: "doc", + Content: []*Node{ + { + NodeType: NodeParagraph, + Content: []*Node{ + {NodeType: ChildNodeText, NodeValue: NodeValue{Text: "a c"}}, + { + NodeType: ChildNodeText, + NodeValue: NodeValue{Text: "d f", Marks: []MarkNode{{MarkType: MarkCode}}}, + }, + }, + }, + { + NodeType: NodeCodeBlock, + Content: []*Node{{NodeType: ChildNodeText, NodeValue: NodeValue{Text: "g i"}}}, + }, + }, + } + + // Escaping is what keeps the markdown renderer from eating the angle + // brackets as an HTML tag, so it is only correct outside of code, which + // the renderer prints verbatim. + out := NewTranslator(&doc, NewMarkdownTranslator(WithMarkdownASCII())).Translate() + + assert.Contains(t, out, `a \ c`) + assert.Contains(t, out, "`d f`") + assert.Contains(t, out, "g i") +} + func TestADFReplaceAll(t *testing.T) { data, err := os.ReadFile("./testdata/md.json") assert.NoError(t, err) diff --git a/pkg/adf/markdown.go b/pkg/adf/markdown.go index ba51cdea..4634f539 100644 --- a/pkg/adf/markdown.go +++ b/pkg/adf/markdown.go @@ -24,6 +24,11 @@ type MarkdownTranslator struct { } openHooks nodeTypeHook closeHooks nodeTypeHook + + ascii bool + // code tracks whether we are inside a code block or an inline code mark, + // where the markdown renderer prints the text verbatim. + code bool } // MarkdownTranslatorOption is a functional option for MarkdownTranslator. @@ -65,6 +70,36 @@ func WithMarkdownCloseHooks(hooks nodeTypeHook) MarkdownTranslatorOption { } } +// WithMarkdownASCII limits the markup the translator adds to ASCII, so the +// result stays printable in a non-UTF-8 locale such as LC_ALL=C. +func WithMarkdownASCII() MarkdownTranslatorOption { + return func(tr *MarkdownTranslator) { + tr.ascii = true + } +} + +// Sanitize implements TextSanitizer interface. +// +// Angle brackets can't be passed through as-is because the markdown renderer +// would consume them as an HTML tag, dropping the text. The default translator +// swaps in lookalike glyphs; in ASCII mode we backslash-escape them instead, +// which renders as the original character. Inside code, where the renderer +// prints text verbatim, neither is needed nor wanted. +func (tr *MarkdownTranslator) Sanitize(s string) string { + if !tr.ascii { + return sanitize(s) + } + + s = strings.TrimSpace(s) + s = strings.TrimRight(s, "\n") + if tr.code { + return s + } + s = strings.ReplaceAll(s, "<", `\<`) + s = strings.ReplaceAll(s, ">", `\>`) + return s +} + // Open implements TagOpener interface. // //nolint:gocyclo @@ -81,6 +116,7 @@ func (tr *MarkdownTranslator) Open(n Connector, _ int) string { tag.WriteString("> ") case NodeCodeBlock: tag.WriteString("```") + tr.code = true nl := true if attrs != nil { @@ -138,13 +174,18 @@ func (tr *MarkdownTranslator) Open(n Connector, _ int) string { case InlineNodeMention: tag.WriteString(" @") case InlineNodeCard: - tag.WriteString(" 📍 ") + if tr.ascii { + tag.WriteString(" ") + } else { + tag.WriteString(" 📍 ") + } case MarkStrong: tag.WriteString(" **") case MarkEm: tag.WriteString(" _") case MarkCode: tag.WriteString(" `") + tr.code = true case MarkStrike: tag.WriteString(" -") case MarkLink: @@ -173,6 +214,7 @@ func (tr *MarkdownTranslator) Close(n Connector) string { tag.WriteString("\n") case NodeCodeBlock: tag.WriteString("\n```\n") + tr.code = false case NodePanel: tag.WriteString("---\n") case NodeHeading: @@ -215,6 +257,7 @@ func (tr *MarkdownTranslator) Close(n Connector) string { tag.WriteString("_ ") case MarkCode: tag.WriteString("` ") + tr.code = false case MarkStrike: tag.WriteString("- ") case MarkLink: From a8b3360a3de2f47aa8beb0b0759b7784ee301fe3 Mon Sep 17 00:00:00 2001 From: Reto Date: Sun, 12 Jul 2026 15:12:24 +0200 Subject: [PATCH 2/2] fix: Treat non-TTY output as plain all the way down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Render() already routed dumb terminals and pipes through renderPlain, but only the glamour style followed: the header and the ADF translator branch on Display.Plain, which is set solely by the --plain flag. Piping without the flag thus mixed ASCII bullets with emoji headers and Unicode ADF decorations. Setting the flag on the (value) receiver before rendering makes every layer agree. Also pin the one leak the renderer can't avoid: glamour truncates a table cell's link URL with a hardcoded ellipsis (ansi/table_links.go), out of reach of the style config. The new test documents it as a known issue and will flag when a glamour upgrade changes the behavior. Getting the leak to reproduce needs a real table — the ADF translator only writes pipes between cells, so a one-column table is never parsed as one. --- internal/view/issue.go | 3 +++ internal/view/issue_test.go | 54 +++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/internal/view/issue.go b/internal/view/issue.go index 7ab7806e..c892f9ab 100644 --- a/internal/view/issue.go +++ b/internal/view/issue.go @@ -56,6 +56,9 @@ type Issue struct { // Render renders the view. func (i Issue) Render() error { if i.Display.Plain || tui.IsDumbTerminal() || tui.IsNotTTY() { + // A dumb terminal or a pipe needs the same ASCII-only treatment as an + // explicit --plain; everything below renderPlain branches on this flag. + i.Display.Plain = true return i.renderPlain(os.Stdout) } r, err := MDRenderer() diff --git a/internal/view/issue_test.go b/internal/view/issue_test.go index 255fc23f..99043251 100644 --- a/internal/view/issue_test.go +++ b/internal/view/issue_test.go @@ -2,6 +2,7 @@ package view import ( "bytes" + "strings" "testing" "unicode" @@ -446,6 +447,59 @@ func TestPlainIssueViewKeepsAngleBrackets(t *testing.T) { assert.Contains(t, out, "if (ac) {}") } +// Known issue: a link inside a table whose URL is longer than the terminal +// width still leaks one non-ASCII rune into plain output. Glamour truncates +// the URL with a hardcoded "…" (ansi/table_links.go), which the style config +// cannot override, so plainMDRenderer can't get rid of it. Fixable, but only +// by post-processing the rendered output or patching glamour — not worth it +// so far. This test pins the leak so we notice when a glamour upgrade or a +// workaround changes the behavior; if it starts failing with no non-ASCII +// left, delete it and celebrate. +func TestPlainIssueViewLongTableLinkLeaksEllipsis(t *testing.T) { + t.Parallel() + + cell := func(kind string, text string, marks []adf.MarkNode) *adf.Node { + return &adf.Node{ + NodeType: adf.NodeType(kind), + Content: []*adf.Node{ + { + NodeType: "paragraph", + Content: []*adf.Node{{NodeType: "text", NodeValue: adf.NodeValue{Text: text, Marks: marks}}}, + }, + }, + } + } + link := []adf.MarkNode{{ + MarkType: "link", + Attributes: map[string]any{"href": "https://test.local/" + strings.Repeat("a", 200)}, + }} + + data := decoratedIssue() + data.Fields.Description = &adf.ADF{ + Version: 1, + DocType: "doc", + Content: []*adf.Node{ + { + NodeType: "table", + Content: []*adf.Node{ + {NodeType: "tableRow", Content: []*adf.Node{cell("tableHeader", "col a", nil), cell("tableHeader", "col b", nil)}}, + {NodeType: "tableRow", Content: []*adf.Node{cell("tableCell", "a link", link), cell("tableCell", "plain", nil)}}, + }, + }, + }, + } + + issue := Issue{ + Server: "https://test.local", + Data: data, + Display: DisplayFormat{Plain: true}, + } + + var b bytes.Buffer + assert.NoError(t, issue.renderPlain(&b)) + assert.Equal(t, []string{"…"}, nonASCII(b.String())) +} + func TestIssueViewKeepsDecorationsWhenNotPlain(t *testing.T) { t.Parallel()