Skip to content

fix: try to enable vt or disable colors on startup on Windows#383

Merged
zeroshade merged 3 commits into
columnar-tech:mainfrom
amoeba:fix/windows-ansi-sequences
May 21, 2026
Merged

fix: try to enable vt or disable colors on startup on Windows#383
zeroshade merged 3 commits into
columnar-tech:mainfrom
amoeba:fix/windows-ansi-sequences

Conversation

@amoeba
Copy link
Copy Markdown
Member

@amoeba amoeba commented May 19, 2026

Fixes an issue a user reported privately where they were seeing ANSI escapes in dbc's output. I was able to reproduce their issue by setting HKCU\Console\VirtualTerminalLevel to 1 and I came up with this as a fix. When dbc starts on Windows, try to enable VT processing if it's not on and, if we fail, set NO_COLOR so the user doesn't get garbled output. I wasn't able to figure out how an admin could disable VT processing so that branch wasn't tested.

@amoeba amoeba requested a review from zeroshade May 19, 2026 02:35
@amoeba amoeba changed the title fix: try to enable vt or disable colors on startup fix: try to enable vt or disable colors on startup on Windows May 19, 2026

// Check if VT processing is already enabled
if mode&ENABLE_VIRTUAL_TERMINAL_PROCESSING != 0 {
return true
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a normal system, we always hit this return.

Comment thread cmd/dbc/main.go Outdated
)

// Attempt to enable VT processing or disable colors if we can't
initWindowsConsole()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we're differentiating this between systems using go:build can we pick a better name for this? like initVTProcessing or something like that?

Comment thread cmd/dbc/windows_console.go Outdated
@@ -0,0 +1,72 @@
//go:build windows
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this goes below the copyright header but above the package line. It just needs to have empty lines surrounding it.

Alternatively, if the filename just ends with _windows.go then you don't even actually need this line 😄

Comment thread cmd/dbc/windows_console.go Outdated
Comment thread cmd/dbc/windows_console_unix.go Outdated
@@ -0,0 +1,19 @@
//go:build !windows
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this down below the copyright header but above the package line. it just needs to be surrounded by empty lines. See

//go:build !windows
for example

@zeroshade
Copy link
Copy Markdown
Member

Just a few nitpicks 😄

amoeba and others added 2 commits May 20, 2026 10:54
Co-authored-by: Matt Topol <zotthewizard@gmail.com>
@amoeba
Copy link
Copy Markdown
Member Author

amoeba commented May 20, 2026

I reorganized things in 0c9b0cb. How does that look?

@amoeba amoeba requested a review from zeroshade May 20, 2026 18:01
@zeroshade
Copy link
Copy Markdown
Member

LGTM

@zeroshade zeroshade merged commit 3de7421 into columnar-tech:main May 21, 2026
13 checks passed
zeroshade added a commit that referenced this pull request May 21, 2026
Resolves conflict in cmd/dbc/main.go: ports the initVTProcessing() call
added on main (#383) into the new main() that calls runStartup(), instead
of the pre-refactor main() body that no longer exists on this branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants