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

Render ASCII-only spinner and icons on conhost.exe #392

Merged
merged 1 commit into from
Jun 14, 2024
Merged

Conversation

DavidS-ovm
Copy link
Contributor

@DavidS-ovm DavidS-ovm commented Jun 13, 2024

Fixes #388

@DavidS-ovm
Copy link
Contributor Author

Refactored the IsConhost() logic for more linear flow.

@dylanratcliffe please give this a spin on your windows machine.

Copy link
Member

@dylanratcliffe dylanratcliffe left a comment

Choose a reason for hiding this comment

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

The logic is working well, we need to replace the spinner too of course but the logic works

@DavidS-ovm
Copy link
Contributor Author

Added the missing call to PlatformSpinner() in tea_taskmodel.go

Copy link
Member

@dylanratcliffe dylanratcliffe left a comment

Choose a reason for hiding this comment

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

Logic looks good, it will of course also need to compile on Darwin, which the current _linux and _windows doesn't allow. Given that we're not doing anything at the compiler level that doesn't work cross-platform (like making certain syscalls) I'd be tempted to just compile all the logic in rather than doing it per-version, it's only a few lines of code but up to you

@DavidS-ovm
Copy link
Contributor Author

adding theme_darwin.go is so much easier than trying to divine whether there's a /prov/version and dealing with random environment variables.

@DavidS-ovm DavidS-ovm merged commit 11d3c6c into main Jun 14, 2024
5 checks passed
@DavidS-ovm DavidS-ovm deleted the conhost branch June 14, 2024 09:00
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.

Spinners don't render in Powershell or Command Prompt
2 participants