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..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() @@ -179,6 +182,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 +298,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 +449,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 +499,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..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" @@ -350,19 +351,153 @@ 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) {}") +} + +// 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) { 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: