feat: Adds ANSI color support for Windows terminals #39

Andrei-hub11 · 2025-07-10T18:54:55Z

This PR introduces ANSI color support for Windows terminals and ensures
dump output does not pollute automated scripts or log files.

Enables colored output on Windows terminals via ansi_windows.go.
Adds TTY detection to disable colors when output is not a terminal.
Integration tests now verify that dumps in automated scripts are clean.

Before:

After:

codecov · 2025-07-10T20:49:51Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

ccoVeille

I would move these terminal/ansi file to a dedicated internal package

I'm a bit afraid it would add complexity to someone reading the code. You could then add a README or godoc package explaining why this solution was preferred to known lib like isatty or /x/terms

I'm fine with your idea, I just think it should be moved in a dedicated place

ccoVeille · 2025-07-11T04:05:56Z

terminal_unix.go

+	"testing"
+)
+
+var isTestEnv = testing.Testing


Please add a comment about the fact this variable exist to be overwritten in test if needed for test purpose.

Also please consider to move it to a new file named terminal.go that might only contain this variable.

You are repeating code in terminal_unix.go terminal_windows.go

Andrei-hub11 · 2025-07-11T06:38:06Z

I would move these terminal/ansi file to a dedicated internal package

I'm a bit afraid it would add complexity to someone reading the code. You could then add a README or godoc package explaining why this solution was preferred to known lib like isatty or /x/terms

I'm fine with your idea, I just think it should be moved in a dedicated place

@ccoVeille Yeah, you might be right. I initially intended to leave it as a draft and ask for your help with it, because the logic is indeed complex, even though it’s not many lines and other libraries do something similar to support Windows. Also, even if we decide to move the support to an internal package, we would still need to keep the check to know whether the output is being redirected to a file, so that ANSI codes aren’t added where they would only create noise.

internal/terminal/terminal_unix_test.go

internal/terminal/terminal_windows.go

ccoVeille · 2025-07-14T07:07:02Z

internal/terminal/terminal_windows_test.go

+func TestIsTerminal_Windows(t *testing.T) {
+	// Mock IsTestEnv to bypass the test environment check
+	originalIsTestEnv := IsTestEnv
+	IsTestEnv = func() bool { return false }
+	defer func() { IsTestEnv = originalIsTestEnv }()


Please consider this

Suggested change

func TestIsTerminal_Windows(t *testing.T) {

// Mock IsTestEnv to bypass the test environment check

originalIsTestEnv := IsTestEnv

IsTestEnv = func() bool { return false }

defer func() { IsTestEnv = originalIsTestEnv }()

// MockIsTestEnv bypasses the test environment check

func MockIsTestEnv(t *testing. T, value bool) {

t.Helper ()

originalIsTestEnv := IsTestEnv

IsTestEnv = func() bool { return value }

t.Cleanup(func() { IsTestEnv = originalIsTestEnv })

}

func TestIsTerminal_Windows(t *testing.T) {

MockIsTestEnv(t, false)

The mock func could be move to terminal.go

Andrei-hub11 · 2025-07-26T10:40:25Z

@ccoVeille Hey, I refactored the check to use a more meaningful approach: one that relies on a Go-maintained package and properly handles the complexity of detecting terminals.

I also tried writing an integration-style test like this:

t.Run("should return true for a ConPTY", func(t *testing.T) {
    ptm, err := pty.Start(exec.Command("cmd.exe", "/c", "exit"))
    if err != nil {
        if err.Error() == "unsupported" {
            t.Skip("ConPTY not supported on this version of Windows")
        }
        t.Fatalf("failed to start pty: %v", err)
    }
    defer ptm.Close()
    assert.True(t, isTerminal(ptm), "isTerminal should be true for a ConPTY")
})

…but honestly, it didn’t feel quite right, and was actually problematic in practice. So I ended up removing it. Do you happen to see a better way to test this?

ccoVeille · 2025-07-26T15:52:42Z

godump_integration_test.go

I wouldn't add integration test.

The lib you import are tested.

The only thing you need I think is to test you the behavior of the code when you overload the result of IsTerminal

Exactly. We're on the same page. I just got the impression you thought I was still using a mock for that.

ccoVeille · 2025-07-26T15:54:19Z

terminal_test.go

+		tmpFile, err := os.CreateTemp("", "test-is-terminal")
+		assert.NoError(t, err)
+		defer os.Remove(tmpFile.Name())
+		defer tmpFile.Close()


This is enough

Suggested change

tmpFile, err := os.CreateTemp("", "test-is-terminal")

assert.NoError(t, err)

defer os.Remove(tmpFile.Name())

defer tmpFile.Close()

tmpFile, err := os.CreateTemp(t.TmpDir(), "test-is-terminal")

assert.NoError(t, err)

Please consider this

ccoVeille · 2025-07-26T16:00:39Z

internal/ansi/ansi_windows.go

I don't get the purpose of this.

It's a package that does things, OK.

The Enable is called via an init when using windows, OK.

But then?

I mean how does that interact or has effect of the fact godump calls term.IsTerminal ?

I feel like the windowansi package and its initialization leads to something that is not used.

What am I missing?

windowsansi.Enable() enables ANSI color support on Windows terminals. On the other hand, IsTerminal ensures that color output is only shown when the output is actually a terminal. For example, it prevents color codes from appearing when output is redirected to a file.

So while they handle different concerns, they work together: one enables support, the other controls when it's appropriate to use it.

I feel they should in the same module then

I mean the init call to windowansi.Enable made for window module could be in the terminal module

terminal.go

Akkadius · 2025-08-11T04:50:01Z

Where is this at ? I see a lot of open threads

Akkadius · 2025-08-11T05:03:11Z

Also, from what I understand, ANSI support is enabled by default in newer versions of windows and terminals (needs verification). If that is the case it'd be ideal to not have to do all of this.

Andrei-hub11 · 2025-08-11T10:35:26Z

It’s true that recent versions of Windows have ANSI support, but it’s not always enabled in the console or in all situations. In CMD, some built-in commands already support ANSI without needing to enable anything. The attached images show this behavior. In my case, I had to enable it manually. The fatih/color project, written in Go, also had to implement something similar to enable ANSI support on Windows.

cmilesdev · 2025-10-15T03:52:47Z

@ccoVeille since you handled a lot of the review here - what's your take on where this PR stands ?

ccoVeille · 2025-10-15T06:08:51Z

internal/windowsansi/winansi_test.go

+	ret, _, err := syscall.NewLazyDLL("kernel32.dll").NewProc("GetConsoleMode").Call(uintptr(handle), uintptr(unsafe.Pointer(&mode)))
+	if ret == 0 {
+		// In Go 1.16+, err is not nil on failure. On older versions, it might be.
+		// So we return the error from the syscall call directly.
+		return 0, err
+	}
+	return mode, nil


There is something I don't get either with the code or comment.

It's a question of logic.

The comment is "In Go 1.16+, err is not nil on failure. On older versions, it might be."

And here is the code

ret, _, err := syscall.NewLazyDLL("kernel32.dll").NewProc("GetConsoleMode").Call(uintptr(handle), uintptr(unsafe.Pointer(&mode))) if ret == 0 { // In Go 1.16+, err is not nil on failure. On older versions, it might be. // So we return the error from the syscall call directly. return 0, err } return mode, nil

The comment is about having a nil error on failure for version prior Go 1.16

So the code should be this, no ?

Suggested change

ret, _, err := syscall.NewLazyDLL("kernel32.dll").NewProc("GetConsoleMode").Call(uintptr(handle), uintptr(unsafe.Pointer(&mode)))

if ret == 0 {

// In Go 1.16+, err is not nil on failure. On older versions, it might be.

// So we return the error from the syscall call directly.

return 0, err

}

return mode, nil

ret, _, err := syscall.NewLazyDLL("kernel32.dll").NewProc("GetConsoleMode").Call(uintptr(handle), uintptr(unsafe.Pointer(&mode)))

if err != nil {

return 0, err

}

if ret == 0 {

// For Go versions older than 1.16, err could be nil on failure

return 0, errors.New("blah blah")

}

return mode, nil

If the comment is wrong, and the code behaving like an error might be returned on success. Then the code should be this

ret, _, err := syscall.NewLazyDLL("kernel32.dll").NewProc("GetConsoleMode").Call(uintptr(handle), uintptr(unsafe.Pointer(&mode))) if err != nil && ret != 0 { return 0, err } return mode, nil

The documentation says:

“The returned error is always non-nil, constructed from the result of GetLastError. Callers must inspect the primary return value to decide whether an error occurred (according to the semantics of the specific function being called) before consulting the error.”

But since the comment was a bit confusing anyway, I’ve updated it.

I checked kernel32 package it's indeed far from being idiomatic or obvious. Your code is fine

ccoVeille · 2025-10-15T06:18:42Z

internal/ansi/ansi_windows_test.go

+	if ret == 0 {
+		return err
+	}
+	return nil


Here also, there is something to do, at least a comment

ccoVeille · 2025-10-15T06:21:43Z

internal/ansi/ansi_windows.go

+	// GetConsoleMode fails if not in a real console.
+	ret, _, _ := procGetConsoleMode.Call(uintptr(handle), uintptr(unsafe.Pointer(&mode)))
+	if ret == 0 {
+		return
+	}


The error is ignored, it could be returned via Enable() error

It would be cleaner. Even if for now, Enable is called via init that would ignore it.

See a suggestion I'm doing here

#39 (comment)

ccoVeille · 2025-10-15T06:22:43Z

terminal_test.go

+		tmpFile, err := os.CreateTemp("", "test-is-terminal")
+		assert.NoError(t, err)
+		defer os.Remove(tmpFile.Name())
+		defer tmpFile.Close()


Please consider this

ccoVeille · 2025-10-15T06:28:12Z

ansi_windows.go

I would remove this file, and rework the windowsansi package to something like this.

internal/ansi/ansi_windows.go

//go:build windows package ansi // remove the init thing func Enable() error { // Then the content of internal/windowsansi/winansi.go return nil }

internal/ansi/ansi_others.go

//go:build !windows package ansi func Enable() error { return nil }

Then call ansi.Enable from godump when needed. Then you can catch the error returned by the DDL methods

But if the windows lib require to use init, I would do this

internal/fixansi/ansi_windows.go

//go:build windows package fixansi func init() { _ = enable() } // enable is here to be able to test it func enable() error { // Then the content of internal/windowsansi/winansi.go }

And then

In godump, I would use this

import ( // ... _ internal/fixansi )

I would prefer the first option, or the exiting code currently in the PR

…r output when stdout is not a TTY

Andrei-hub11 · 2025-10-15T11:06:37Z

@ccoVeille see if it’s better now or if I misunderstood any of your comments.

ccoVeille · 2025-10-15T12:39:11Z

internal/ansi/ansi_windows.go

+
+	// GetConsoleMode fails if not in a real console.
+	ret, _, err := procGetConsoleMode.Call(uintptr(handle), uintptr(unsafe.Pointer(&mode)))
+	if ret == 0 {


This no ?

Suggested change

if ret == 0 {

if ret != 0 {

No. Take a look:

"If the function succeeds, the return value is nonzero. If the function fails, the return value is zero."

ccoVeille · 2025-10-15T12:39:29Z

internal/ansi/ansi_windows.go

+
+	// Try to set the new console mode.
+	ret, _, err = procSetConsoleMode.Call(uintptr(handle), uintptr(newMode))
+	if ret == 0 {


Suggested change

if ret == 0 {

if ret != 0 {

ccoVeille · 2025-10-15T12:41:27Z

internal/ansi/ansi_windows.go

+
+	// GetConsoleMode fails if not in a real console.
+	ret, _, err := procGetConsoleMode.Call(uintptr(handle), uintptr(unsafe.Pointer(&mode)))
+	if ret == 0 {


I would use a constant

something like

const syscallNoError = 0

It would make it clearer when comparing to 0

ccoVeille · 2025-10-15T12:43:33Z

@ccoVeille see if it’s better now or if I misunderstood any of your comments.

Thanks for doing the change. I hope you found interest in the suggestions I made

ccoVeille · 2025-10-15T12:48:35Z

internal/ansi/ansi_windows.go

+
+	// GetConsoleMode fails if not in a real console.
+	ret, _, err := procGetConsoleMode.Call(uintptr(handle), uintptr(unsafe.Pointer(&mode)))
+	if ret == 0 {


What about something like this?

Maybe it would be simpler to read, and more idiomatic

ret, _, err := procGetConsoleMode.Call(uintptr(handle), uintptr(unsafe.Pointer(&mode))) if err != nil && ret != syscallNoError { return err }

I think the previous answer explains it.

Andrei-hub11 · 2025-10-15T21:08:57Z

Thanks for doing the change. I hope you found interest in the suggestions I made

Yeah, I did. And I appreciate it. Take a new look at the forced commit.

@Akkadius, add @ccoVeille as co-author if it gets approved at some point

'---------

Co-authored-by: ccoVeille [email protected]'

Andrei-hub11 · 2025-10-15T21:20:05Z

It works very well here:

go run demo/main.go > "C:\path\to\demo_output.txt"
2025/10/15 18:13:13 Warning: Failed to enable ANSI support: Invalid identifier.

Output (demo_output.txt):

--- godump.Dump(user) ---
<#dump // demo\main.go:35
User: Alice #main.User

--- output := godump.DumpStr(user) ---
<#dump // demo\main.go:39
User: Alice #main.User


--- html := godump.DumpHTML(user) ---
<div style='background-color:black;'><pre style="background-color:black; color:white; padding:5px; border-radius: 5px">
<span style="color:#999"><#dump // demo\main.go:44</span>
<span style="color:#80ff80">User: Alice</span><span style="color:#999"> #main.User</span>
</pre></div>

--- godump.DumpJSON(user) ---
{
  "Name": "Alice",
  "Profile": {
    "Age": 30,
    "Email": "[email protected]"
  }
}

ccoVeille · 2025-10-16T19:33:08Z

add @ccoVeille as co-author if it gets approved at some point

'---------

Co-authored-by: ccoVeille [email protected]'

You know that as the commit author you can do it

Take a look at your last commit message

refactor: unify windowsansi into ansi

You simply have to edit it like this

refactor: unify windowsansi into ansi

Co-authored-by: ccoVeille <[email protected]>

I'm not even sure Akkadius can do it for you without altering your commit in this exact way.

ccoVeille

LGTM 👍

A minor comment anyway

ansi_windows.go

Andrei-hub11 · 2025-10-16T21:01:31Z

I'm not even sure Akkadius can do it for you without altering your commit in this exact way.

Actually, he can include that in the merge commit without any problem, but I’ll go ahead and redo the commit message. That’s totally fine

Co-authored-by: ccoVeille <[email protected]>

Andrei-hub11 force-pushed the feat/windows-ansi branch 3 times, most recently from 41020b5 to 3c38e93 Compare July 10, 2025 20:49

Andrei-hub11 force-pushed the feat/windows-ansi branch 2 times, most recently from 022bde1 to ae774fa Compare July 11, 2025 01:57

ccoVeille reviewed Jul 11, 2025

View reviewed changes

Andrei-hub11 force-pushed the feat/windows-ansi branch from ae4f99b to 58045bf Compare July 11, 2025 16:38

ccoVeille reviewed Jul 12, 2025

View reviewed changes

internal/terminal/terminal_unix_test.go Outdated Show resolved Hide resolved

internal/terminal/terminal_windows.go Outdated Show resolved Hide resolved

ccoVeille mentioned this pull request Jul 13, 2025

chore: remove enableColor and colorize local variables #38

Merged

ccoVeille reviewed Jul 14, 2025

View reviewed changes

Andrei-hub11 force-pushed the feat/windows-ansi branch from 58045bf to a2a43d2 Compare July 26, 2025 10:28

Andrei-hub11 requested a review from ccoVeille July 26, 2025 10:40

ccoVeille reviewed Jul 26, 2025

View reviewed changes

ccoVeille reviewed Oct 15, 2025

View reviewed changes

Andrei-hub11 added 3 commits October 15, 2025 07:05

feat: Adds ANSI color support for Windows terminals and prevents colo…

e52d8fd

…r output when stdout is not a TTY

fix: CI

22e2634

refactor: streamline ANSI support initialization for Windows terminals

b22f972

Andrei-hub11 force-pushed the feat/windows-ansi branch from a2a43d2 to b456762 Compare October 15, 2025 11:01

ccoVeille reviewed Oct 15, 2025

View reviewed changes

Andrei-hub11 force-pushed the feat/windows-ansi branch from b456762 to 0fb5b1c Compare October 15, 2025 20:59

ccoVeille approved these changes Oct 16, 2025

View reviewed changes

ansi_windows.go Outdated Show resolved Hide resolved

Andrei-hub11 force-pushed the feat/windows-ansi branch from 0fb5b1c to 3e67219 Compare October 16, 2025 21:01

refactor: unify windowsansi into ansi

345aae0

Co-authored-by: ccoVeille <[email protected]>

Andrei-hub11 force-pushed the feat/windows-ansi branch from 3e67219 to 345aae0 Compare October 17, 2025 11:17

feat: Adds ANSI color support for Windows terminals #39

Are you sure you want to change the base?

feat: Adds ANSI color support for Windows terminals #39

Uh oh!

Conversation

Andrei-hub11 commented Jul 10, 2025

Uh oh!

codecov bot commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ccoVeille left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andrei-hub11 commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andrei-hub11 commented Jul 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andrei-hub11 Jul 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Akkadius commented Aug 11, 2025

Uh oh!

Akkadius commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Andrei-hub11 commented Aug 11, 2025

Uh oh!

cmilesdev commented Oct 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andrei-hub11 commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccoVeille commented Oct 15, 2025

Uh oh!

codecov bot commented Jul 10, 2025 •

edited

Loading

Andrei-hub11 commented Jul 11, 2025 •

edited

Loading

Andrei-hub11 Jul 26, 2025 •

edited

Loading

Akkadius commented Aug 11, 2025 •

edited

Loading

Andrei-hub11 commented Oct 15, 2025 •

edited

Loading