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

Spinners don't render in Powershell or Command Prompt #388

Closed
dylanratcliffe opened this issue Jun 11, 2024 · 7 comments · Fixed by #392
Closed

Spinners don't render in Powershell or Command Prompt #388

dylanratcliffe opened this issue Jun 11, 2024 · 7 comments · Fixed by #392
Assignees
Labels
enhancement New feature or request

Comments

@dylanratcliffe
Copy link
Member

dylanratcliffe commented Jun 11, 2024

Screenshot 2024-06-11 at 2 56 40 PM

The fonts that are used here don't seem to support the emojis. If you copy-paste into something else it works fine, so i'm assuming it's the fonts, we will have to switch so something mega-basic when we are on these shells.

Here's where this is printed in code:

cli/cmd/tea_taskmodel.go

Lines 121 to 123 in 0c0fd07

label = lipgloss.NewStyle().Foreground(ColorPalette.BgSuccess).Render("✔︎")
case taskStatusError:
label = lipgloss.NewStyle().Foreground(ColorPalette.BgDanger).Render("✗")

@dylanratcliffe dylanratcliffe added the enhancement New feature or request label Jun 11, 2024
@DavidS-ovm
Copy link
Contributor

PowerShell community discord confirmed multiple times that the default terminal (conhost) won't display emojis. The new Windows Terminal can, but is an optional component.

@dylanratcliffe I'm worried that WSL also might run in conhost (what else would it be) and thus using the linux build won't work either. Can you check that on your windows instance?

@dylanratcliffe dylanratcliffe self-assigned this Jun 12, 2024
@dylanratcliffe
Copy link
Member Author

Confirmed that WSL also doesn't have good fonts:

Screenshot 2024-06-12 at 9 41 54 AM

@dylanratcliffe
Copy link
Member Author

We can detect that we are running under WSL like so:

$ cat /proc/version
Linux version 4.4.0-19041-Microsoft ([email protected]) (gcc version 5.4.0 (GCC) ) #4355-Microsoft Thu Apr 12 17:37:00 PST 2024

@dylanratcliffe
Copy link
Member Author

Screenshot 2024-06-12 at 9 52 31 AM

Windows terminal works great through and can be determined by checking the value of the WT_SESSION env var:

> $env:WT_SESSION
ded037c6-04c4-4a69-a9e7-ab55ecb7b165

@dylanratcliffe
Copy link
Member Author

So in summary we have the following:

  • Any linux terminal: Fine ✅
  • Powershell: Terrible 😢
  • CMD: Terrible 😢
  • Windows Terminal: Fine ✅
  • WSL: Terrible 😢 (However the linux-based packages do work)

So I think the sanest thing we can do is try to detect the correct spinners to use:

  • IF linux
    • IF /proc/version contains Microsoft: Boring spinners
    • ELSE cool spinners
  • IF Windows
    • IF env var WT_SESSION is set: Cool spinners
    • ELSE: Boring spinners

@DavidS-ovm
Copy link
Contributor

DavidS-ovm commented Jun 12, 2024

I've poked around a bit at conhost (the default windows terminal emulator) and Windows Terminal (the shiny new thing) and there's only pain and suffering down this path. See also: microsoft/terminal#7434

When using Windows Terminal, WT_SESSION also exists in WSL processes, so the amended decision tree is: WT_SESSION is set || (is linux && ! WSL)

Of course this still can cause false positives (when conhost is loaded from WT and has WT_SESSION set without the capabilities), but clearly the Windows Terminal folks are not interested in solving this problem, so we're out of luck then.

@dylanratcliffe
Copy link
Member Author

dylanratcliffe commented Jun 12, 2024

Screenshot 2024-06-12 at 8 07 54 PM

I've just checked and Powershell, WSL and even CMD all render fine when run in the Windows Terminal, so seems that if we have a WT_SESSION it's always fine, then we only need to go to basic for the others

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants