Simplify markdown codeblock tests, glamour helper

This commit is focused on PR feedback around codeblock testing and simplifying related code:

1. Use of new `WithOptions(...TermRendererOption) TermRendererOption` to clean up `WithTheme()`

   The `glamour` TermRendererOption pattern has a limitation that users cannot compose multiple options without duplicating code or building one-off anonymous functions.

   However, this commit takes advantage of an enhancement in cli/glamour#3 that allows users to leverage a helper to avoid building one-offs.

1. Use of new `leaanthony/go-ansi-parser` dependency for parsing ANSI escape sequences and display attributes

   In v1 of `markdown_test.go`, the codeblock tests were very simple, testing the result of output of markdown rendering against a string of ANSI escape sequences. The concern raised is that this was testing the result rather than behavior wanted.

   In v2 of `markdown_test.go`, the codeblock tests were refactored to use regex to extract and analyze ANSI escape sequences and display attributes. The concern raised was that this was a lot of logic to build and maintain and might benefit from a dependency that could do it.

   In v3 of `markdown_test.go`, a combination of v1 and v2 approaches for 1) testing that theme appropriate colors are used and 2) testing that ensures accessible display options are used when accessible experience is enabled

Author: andyfeller

go.mod

@@ -5,14 +5,16 @@ go 1.21
 require (
 	github.com/AlecAivazis/survey/v2 v2.3.7
 	github.com/MakeNowJust/heredoc v1.0.0
+	github.com/alecthomas/chroma/v2 v2.14.0
 	github.com/charmbracelet/lipgloss v1.0.0
 	github.com/cli/browser v1.3.0
-	github.com/cli/glamour v0.0.0-20250220192152-8544502ccff9
+	github.com/cli/glamour v0.0.0-20250225134531-b1d96ed4a7e1
 	github.com/cli/safeexec v1.0.0
 	github.com/cli/shurcooL-graphql v0.0.4
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/henvic/httpretty v0.0.6
 	github.com/itchyny/gojq v0.12.15
+	github.com/leaanthony/go-ansi-parser v1.6.1
 	github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
 	github.com/muesli/reflow v0.3.0
 	github.com/muesli/termenv v0.15.3-0.20240618155329-98d742f6907a
@@ -26,7 +28,6 @@ require (
 )
 
 require (
-	github.com/alecthomas/chroma/v2 v2.14.0 // indirect
 	github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect
 	github.com/aymerick/douceur v0.2.0 // indirect
 	github.com/charmbracelet/x/ansi v0.8.0 // indirect

go.sum

@@ -26,6 +26,8 @@ github.com/cli/browser v1.3.0 h1:LejqCrpWr+1pRqmEPDGnTZOjsMe7sehifLynZJuqJpo=
 github.com/cli/browser v1.3.0/go.mod h1:HH8s+fOAxjhQoBUAsKuPCbqUuxZDhQ2/aD+SzsEfBTk=
 github.com/cli/glamour v0.0.0-20250220192152-8544502ccff9 h1:fPJNUzG+Au+pIfYx2c5QNngZ3KLj7xAzjutL6efy1x8=
 github.com/cli/glamour v0.0.0-20250220192152-8544502ccff9/go.mod h1:bB4uNJ5F0+nzpGqwlhKEy7tD0PPL0SxNWEwZzG77vMg=
+github.com/cli/glamour v0.0.0-20250225134531-b1d96ed4a7e1 h1:TrV+Bs2RKR/CeodJDRO6GXzIxT7PAOwbuxth8ACcucg=
+github.com/cli/glamour v0.0.0-20250225134531-b1d96ed4a7e1/go.mod h1:bB4uNJ5F0+nzpGqwlhKEy7tD0PPL0SxNWEwZzG77vMg=
 github.com/cli/safeexec v1.0.0 h1:0VngyaIyqACHdcMNWfo6+KdUYnqEr2Sg+bSP1pdF+dI=
 github.com/cli/safeexec v1.0.0/go.mod h1:Z/D4tTN8Vs5gXYHDCbaM1S/anmEDnJb1iW0+EJ5zx3Q=
 github.com/cli/shurcooL-graphql v0.0.4 h1:6MogPnQJLjKkaXPyGqPRXOI2qCsQdqNfUY1QSJu2GuY=
@@ -60,8 +62,12 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
+github.com/leaanthony/go-ansi-parser v1.6.1 h1:xd8bzARK3dErqkPFtoF9F3/HgN8UQk0ed1YDKpEz01A=
+github.com/leaanthony/go-ansi-parser v1.6.1/go.mod h1:+vva/2y4alzVmmIEpk9QDhA7vLC5zKDTRwfZGOp3IWU=
 github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY=
 github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0=
+github.com/matryer/is v1.4.0 h1:sosSmIWwkYITGrxZ25ULNDeKiMNzFSr4V/eqBQP0PeE=
+github.com/matryer/is v1.4.0/go.mod h1:8I/i5uYgLzgsgEloJE1U6xx5HkBQpAZvepWuujKwMRU=
 github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
 github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
 github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg=

pkg/markdown/markdown.go

@@ -39,20 +39,10 @@ func WithTheme(theme string) glamour.TermRendererOption {
 		switch theme {
 		case "light", "dark":
 			if accessible {
-				// Applying multiple glamour.TermRendererOption here requires a wrapper that applies each
-				// within glamour.NewTermRenderer() in Render() below.
-				stylesOption := glamour.WithStyles(AccessibleStyleConfig(theme))
-				chromaOption := glamour.WithChromaFormatter("terminal16")
-
-				return func(tr *glamour.TermRenderer) error {
-					if err := stylesOption(tr); err != nil {
-						return err
-					}
-					if err := chromaOption(tr); err != nil {
-						return err
-					}
-					return nil
-				}
+				return glamour.WithOptions(
+					glamour.WithStyles(AccessibleStyleConfig(theme)),
+					glamour.WithChromaFormatter("terminal16"),
+				)
 			}
 			style = theme
 		default:

pkg/markdown/markdown_test.go

@@ -4,13 +4,13 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"regexp"
-	"strconv"
 	"strings"
 	"testing"
 
 	"github.com/MakeNowJust/heredoc"
+	"github.com/alecthomas/chroma/v2/styles"
 	"github.com/cli/go-gh/v2/pkg/accessibility"
+	ansi "github.com/leaanthony/go-ansi-parser"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -22,25 +22,11 @@ const (
 	customH2_8bitColorSeq       = "\x1b[38;5;61;"
 	magenta_4bitColorSeq        = "\x1b[35;"
 	brightMagenta_4bitColorSeq  = "\x1b[95;"
-
-	// TODO: Include a little more context on SGR including link to https://en.wikipedia.org/wiki/ANSI_escape_code#Select_Graphic_Rendition_parameters for more info
-	// sgrSequencePattern identifies ANSI escape sequences containing display attributes
-	// that affect the color, emphasis, and other aspects of displaying text.
-	sgrSequencePattern = `\x1b\[(.+?)m`
-
-	// sgrAttributePattern analyzes separate display attributes within an ANSI escape sequence
-	// for detecting color depth (3-bit, 4-bit, 8-bit, 24-bit, etc) or other effects.
-	//
-	// This is a separate regex from sgrSequencePattern as `FindAllStringSubmatch()` does not
-	// handle repeating capture groups well.
-	sgrAttributePattern = `;?(?P<sequence>\d+)` // TODO: change the `sequence` note; if we aren't actually using the name group, remove it
 )
 
 func Test_Render_Codeblocks(t *testing.T) {
 	t.Setenv("GLAMOUR_STYLE", "")
 
-	sequencesRegex := regexp.MustCompile(sgrSequencePattern)
-	attributesRegex := regexp.MustCompile(sgrAttributePattern)
 	text := heredoc.Docf(`
 		%[1]s%[1]s%[1]sgo
 		package main
@@ -92,64 +78,14 @@ func Test_Render_Codeblocks(t *testing.T) {
 
 			out, err := Render(tt.text, WithTheme(tt.theme))
 			require.NoError(t, err)
-			sequences := sequencesRegex.FindAllStringSubmatch(out, -1)
-			require.NotEmpty(t, sequences, "Failed to find expected SGR sequences in rendered output")
-
-			// TODO: Review use of a module like https://github.com/leaanthony/go-ansi-parser/blob/main/ansi.go#L264 to do the sequence and attribute parsing such that we just have to iterate over the results
-			for _, sequence := range sequences {
-				attributes := attributesRegex.FindAllStringSubmatch(sequence[1], -1)
-				require.NotEmpty(t, attributes, "Failed to extract SGR attributes for testing")
-
-				// Analysis loop handles index incrementing due to unique display attribute situations like color depth
-				for i := 0; i < len(attributes); {
-					// TODO: Use constants for 38,48 and maybe remove the int conversion
-					attribute, err := strconv.Atoi(attributes[i][1])
-					require.NoError(t, err, "Failed to convert SGR attribute for testing")
-
-					switch attribute {
-					case 38, 48:
-						// Display attributes for setting 8-bit and 24-bit foreground and background colors
-						colorDepth, err := strconv.Atoi(attributes[i+1][1])
-						require.NoError(t, err, "Failed to convert SGR color depth attribute for testing")
-
-						switch colorDepth {
-						case 2:
-							// 24-bit color display attribute form
-							// - ESC[38;2;⟨r⟩;⟨g⟩;⟨b⟩m for foreground colors
-							// - ESC[48;2;⟨r⟩;⟨g⟩;⟨b⟩m for background colors
-							require.False(t, tt.accessible, "24-bit color is not accessible, customizable")
-
-							color24bitRed, err := strconv.Atoi(attributes[i+2][1])
-							require.NoError(t, err, "Failed to convert 24-bit red color value for testing")
-							require.True(t, color24bitRed >= 0 && color24bitRed <= 255, "24-bit red color value out of 0-255 range")
-
-							color24bitGreen, err := strconv.Atoi(attributes[i+3][1])
-							require.NoError(t, err, "Failed to convert 24-bit green color value for testing")
-							require.True(t, color24bitGreen >= 0 && color24bitGreen <= 255, "24-bit green color value out of 0-255 range")
-
-							color24bitBlue, err := strconv.Atoi(attributes[i+4][1])
-							require.NoError(t, err, "Failed to convert 24-bit blue color value for testing")
-							require.True(t, color24bitBlue >= 0 && color24bitBlue <= 255, "24-bit blue color value out of 0-255 range")
-
-							i += 5
-						case 5:
-							// 8-bit color display attributes form:
-							// - ESC[38;5;⟨n⟩m for foreground colors
-							// - ESC[48;5;⟨n⟩m for background colors
-							require.False(t, tt.accessible, "8-bit color is not accessible, customizable")
-
-							color8bit, err := strconv.Atoi(attributes[i+2][1])
-							require.NoError(t, err, "Failed to convert 8-bit color value for testing")
-							require.True(t, color8bit >= 0 && color8bit <= 255, "8-bit color value out of 0-255 range")
-
-							i += 3
-						default:
-							require.Fail(t, "Unexpected color depth in attribute")
-						}
-					default:
-						// Increment index as this attribute does not affect accessibility currently
-						i += 1
-					}
+
+			styledText, err := ansi.Parse(out)
+			require.NoError(t, err)
+
+			for _, st := range styledText {
+				if tt.accessible {
+					require.Equalf(t, st.ColourMode, ansi.Default, "Inaccessible color found in '%s' at %d", st, st.Offset)
+					require.Falsef(t, st.Faint(), "Inaccessible style found in '%s' at %d", st, st.Offset)
 				}
 			}
 		})
@@ -161,7 +97,12 @@ func Test_Render_Codeblocks(t *testing.T) {
 // match. For more information on ANSI color codes, see
 // https://en.wikipedia.org/wiki/ANSI_escape_code#8-bit
 func Test_Render(t *testing.T) {
-	t.Setenv("GLAMOUR_STYLE", "")
+	codeBlock := heredoc.Docf(`
+		%[1]s%[1]s%[1]sgo
+		fmt.Println("Hello, world!")
+		%[1]s%[1]s%[1]s
+	`, "`")
+
 	tests := []struct {
 		name             string
 		text             string
@@ -209,14 +150,43 @@ func Test_Render(t *testing.T) {
 			accessibleEnvVar: "true",
 			wantOut:          fmt.Sprintf("%s1mh2", brightMagenta_4bitColorSeq),
 		},
+		{
+			name:    "when the light theme is selected, the codeblock renders using 8-bit colors",
+			text:    codeBlock,
+			theme:   "light",
+			wantOut: "\x1b[0m\x1b[38;5;235mfmt\x1b[0m\x1b[38;5;210m.\x1b[0m\x1b[38;5;35mPrintln\x1b[0m\x1b[38;5;210m(\x1b[0m\x1b[38;5;95m\"Hello, world!\"\x1b[0m\x1b[38;5;210m)\x1b[0m",
+		},
+		{
+			name:    "when the dark theme is selected, the codeblock renders using 8-bit colors",
+			text:    codeBlock,
+			theme:   "dark",
+			wantOut: "\x1b[0m\x1b[38;5;251mfmt\x1b[0m\x1b[38;5;187m.\x1b[0m\x1b[38;5;42mPrintln\x1b[0m\x1b[38;5;187m(\x1b[0m\x1b[38;5;173m\"Hello, world!\"\x1b[0m\x1b[38;5;187m)\x1b[0m",
+		},
+		{
+			name:             "when the accessible env var is set and the light theme is selected, the codeblock renders using 4-bit colors",
+			text:             codeBlock,
+			theme:            "light",
+			accessibleEnvVar: "true",
+			wantOut:          "\x1b[0m\x1b[30mfmt\x1b[0m\x1b[33m.\x1b[0m\x1b[36mPrintln\x1b[0m\x1b[33m(\x1b[0m\x1b[90m\"Hello, world!\"\x1b[0m\x1b[33m)\x1b[0m",
+		},
+		{
+			name:             "when the accessible env var is set and the dark theme is selected, the codeblock renders using 4-bit colors",
+			text:             codeBlock,
+			theme:            "dark",
+			accessibleEnvVar: "true",
+			wantOut:          "\x1b[0m\x1b[37mfmt\x1b[0m\x1b[37m.\x1b[0m\x1b[36mPrintln\x1b[0m\x1b[37m(\x1b[0m\x1b[33m\"Hello, world!\"\x1b[0m\x1b[37m)\x1b[0m",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			// Unregister cached chroma style causing codeblock tests to fail based on previous theme
+			delete(styles.Registry, "charm")
 			t.Setenv(accessibility.ACCESSIBILITY_ENV, tt.accessibleEnvVar)
 
-			if tt.styleEnvVar != "" {
-				tmpDir := t.TempDir()
-				path := filepath.Join(tmpDir, fmt.Sprintf("%s.json", tt.styleEnvVar))
+			if tt.styleEnvVar == "" {
+				t.Setenv("GLAMOUR_STYLE", "")
+			} else {
+				path := filepath.Join(t.TempDir(), fmt.Sprintf("%s.json", tt.styleEnvVar))
 				err := os.WriteFile(path, []byte(customGlamourStyle(t)), 0644)
 				if err != nil {
 					t.Fatal(err)