Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: detect windows terminal #141

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Delta456
Copy link

@Delta456 Delta456 commented Jul 1, 2023

This PR adds the functionality to detect WSL (1 and 2) while using Windows Terminal as it supports TrueColor color profile. It also closes the issue #124.

termenv_unix.go Outdated Show resolved Hide resolved
Co-authored-by: Ayman Bagabas <[email protected]>
@aymanbagabas
Copy link
Collaborator

@Delta456 could you please provide unit tests for this case?

@Delta456
Copy link
Author

Delta456 commented Jul 3, 2023

@Delta456 could you please provide unit tests for this case?

Done

termenv_test.go Outdated Show resolved Hide resolved
@aymanbagabas
Copy link
Collaborator

Looking more into this, I don't think we need to check for WSL, we only need to check if we are running Windows Terminal via the WT_SESSION variable and return TrueColor if so.

diff --git a/termenv_unix.go b/termenv_unix.go
index 24d519a5d3d5..3ccf13bd96b4 100644
--- a/termenv_unix.go
+++ b/termenv_unix.go
@@ -57,6 +57,10 @@ func (o *Output) ColorProfile() Profile {
 	}
 
 	if strings.Contains(term, "256color") {
+		// Check if we're running Windows Terminal
+		if os.Getenv("WT_SESSION") != "" {
+			return TrueColor
+		}
 		return ANSI256
 	}
 	if strings.Contains(term, "color") {

@Delta456
Copy link
Author

Delta456 commented Jul 5, 2023

Looking more into this, I don't think we need to check for WSL, we only need to check if we are running Windows Terminal via the WT_SESSION variable and return TrueColor if so.

diff --git a/termenv_unix.go b/termenv_unix.go
index 24d519a5d3d5..3ccf13bd96b4 100644
--- a/termenv_unix.go
+++ b/termenv_unix.go
@@ -57,6 +57,10 @@ func (o *Output) ColorProfile() Profile {
 	}
 
 	if strings.Contains(term, "256color") {
+		// Check if we're running Windows Terminal
+		if os.Getenv("WT_SESSION") != "" {
+			return TrueColor
+		}
 		return ANSI256
 	}
 	if strings.Contains(term, "color") {

I think we should also check WSL just to be double-sure.

And add test to look for WT_SESSION
@aymanbagabas aymanbagabas force-pushed the detect_wsl_inside_wt branch from d840af0 to 2fde0ed Compare July 5, 2023 16:47
@aymanbagabas aymanbagabas requested a review from muesli July 5, 2023 16:50
@aymanbagabas aymanbagabas changed the title termenv: detect wsl inside windows terminal feat: detect windows terminal Jul 5, 2023
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.

3 participants