Ensure codeblocks use accessible colors #183
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,10 @@ import ( | |
| "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" | ||
| ) | ||
|
|
@@ -21,12 +24,85 @@ const ( | |
| brightMagenta_4bitColorSeq = "\x1b[95;" | ||
| ) | ||
|
|
||
| func Test_Render_Codeblocks(t *testing.T) { | ||
| t.Setenv("GLAMOUR_STYLE", "") | ||
|
|
||
| text := heredoc.Docf(` | ||
| %[1]s%[1]s%[1]sgo | ||
| package main | ||
|
|
||
| import ( | ||
| "fmt" | ||
| ) | ||
|
|
||
| func main() { | ||
| fmt.Println("Hello, world!") | ||
| } | ||
| %[1]s%[1]s%[1]s | ||
| `, "`") | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| text string | ||
| theme string | ||
| accessible bool | ||
| }{ | ||
| { | ||
| name: "when the light theme is selected, the codeblock renders using 8-bit colors", | ||
| text: text, | ||
| theme: "light", | ||
| }, | ||
| { | ||
| name: "when the dark theme is selected, the codeblock renders using 8-bit colors", | ||
| text: text, | ||
| theme: "dark", | ||
| }, | ||
| { | ||
| name: "when the accessible env var is set and the light theme is selected, the codeblock renders using 4-bit colors", | ||
| text: text, | ||
| theme: "light", | ||
| accessible: true, | ||
| }, | ||
| { | ||
| name: "when the accessible env var is set and the dark theme is selected, the codeblock renders using 4-bit colors", | ||
| text: text, | ||
| theme: "dark", | ||
| accessible: true, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| if tt.accessible { | ||
| t.Setenv(accessibility.ACCESSIBILITY_ENV, "true") | ||
| } | ||
|
|
||
| out, err := Render(tt.text, WithTheme(tt.theme)) | ||
| require.NoError(t, err) | ||
|
|
||
| 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) | ||
|
Comment on lines
+87
to
+88
There was a problem hiding this comment. Can you explain what exactly these two checks are doing? The context for what this is asserting is hidden behind the dependency here. I'd prefer that context to not requiring spelunking into the Aside: |
||
| } | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Test_Render verifies that the proper ANSI color codes are applied to the rendered | ||
| // markdown by examining the ANSI escape sequences in the output for the correct color | ||
| // 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) { | ||
| 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 | ||
|
|
@@ -74,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") | ||
|
There was a problem hiding this comment. Consider wrapping this in |
||
| t.Setenv(accessibility.ACCESSIBILITY_ENV, tt.accessibleEnvVar) | ||
|
|
||
| if tt.styleEnvVar == "" { | ||
| t.Setenv("GLAMOUR_STYLE", "") | ||
|
There was a problem hiding this comment. Curious as to why this is necessary. |
||
| } 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) | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is WAY better ❤️ I think it's super clear what's happening here and I love it. Thanks!