-
Notifications
You must be signed in to change notification settings - Fork 148
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
Improve Windows Support #5
Comments
I hope you are able to find a way to make this work on Windows. Following. |
Kinda works on windows following this guide: https://askubuntu.com/questions/827952/a-better-terminal-experience-for-windows-subsystem-for-linuxwsl But several emojis still don't show up as expected in the examples. For example 'Loading Unicorn' in persist_spin.py is not displaying correctly. |
Unfortunately, my users don't have and most likely will never have Windows subsystem for Linux. Microsoft really needs to catch up with their terminal interface. |
I think |
The ability to provide fallbacks would be nice, something like this:
|
Could you please provide some information on what spinners ARE currently supported on windows? I'm currently fiddling around with this:
and no matter what spinner I provide for the |
@M451 This happens because I haven't done much testing on Windows, hence only one spinner is enabled as default on Windows platform. If you're on Windows platform, maybe you can help the project with this. Halo depends on py-spinners. I have a script which demonstrates all spinners. If you can check and create a list of spinners supported on Windows, I can whitelist them for Windows. |
Okay so I tested this on Windows 10. First I had to fix some things since the examples still use Python2 code and I'm running from Python3 ( Result: with the default console and/or PowerShell console and the default font (Lucida Console) I was able to have those spinners displayed correctly:
So you should be able to whitelist them without any changes. But this bugged me: why wouldn't the other spinners work? Windows console does support UTF-8 after all. Of course you have to make sure the font you're using also is able to display all those UTF-8 symbols but I'm only using UTF-8 fonts for my consoles so... this SHOULD have worked. I investigated abit and found that the current lack of support is due to the current implementation of the test script. I noticed that lot that caused the lack of support for Windows was fixed by Python 3.3 in pep-0528, see this link I changed the animation method for windows like this and run it with Python 3.6
I also changed the font to a more complete, default Windows UTF-8 font (e.g. NSimSun) and it rendered nearly all chars perfectly. You can see the result attached. Please note: the default Windows font will not work very well even with Python 3.6 because it doesn't many UTF-8 characters. This however should not qualify to not whitelisting them or "silently" falling back to "line" altought the dev specified something else. I'd rather have a quick doc somewhere on what spinners would be safe to use on which platform. Maybe even a simple subclass of possible spinners depending on the OS type since we will not get to a simple cross plattform support anytime soon |
Thank you for such detailed analysis. This will surely help in fixing a lot of the present issues. 💯
Thank you for raising this issue. I've fixed it now. Also, thank you for curating the list of spinners that can run in Windows environment. Here are a few solutions I had thought of earlier related to this:
I wish to support Python 2.7 for as long as possible and yes, there are dependents who make use of this version. We can surely explore this more and fix it for Python 3.3+ users.
I agree. It is wrong to fall silently to "line" instead of informing user and I wish to change this. Let's say I build a CLI for linux platform mainly and a Windows user tries it. Since the spinner is not supported on Windows, shouldn't it fall back to a default spinner without throwing an error? Now let's say we document Windows supported spinners. Someone who is building the software for cross-platform use can surely make use of the document for spinner selection. So, I feel falling silently to "line" spinner is a good option but yes we can surely whitelist known spinners as discussed in point 2. Would you like to raise a PR for this? 😄 |
@manrajgrover I can confirm that the default font (Consolas) still has terrible output: I'm decent at making docs with Sphinx and reStructuredText, if you want them hosted on GitHub or readthedocs.org. I can start them with what's in README.md and add a Windows compatibility page. Although I have more experience with Linux, I have a Windows 10 Pro Insider VM to work on Windows support. In the meantime, I'll work on whitelisting for Windows in _get_spinner() |
@JoseALermaIII That would be really helpful. Currently, all of the documentation is part of README.md as far as I can remember. This will allow better structured documentation and easier access and maintainability with automation.
Looking forward to your PR. Also, apologies for late replies. I'm in a middle of switching roles, hence the delay. |
@manrajgrover No worries. I'm also occupied with other roles. I have a patch that's working, but I'd be glad to work on Sphinx docs after I solve this puzzle. Edit: Seems to be a known issue with |
Microsoft is making a new terminal which will support emoji. It will support cmd, powershell, and bash. |
Nice find! Version 1.0 in Winter 2019. |
All of the spinners appear to work perfectly in both powershell 6.2.2 and cmd in Windows Terminal Preview v0.7.3451.0 on Windows 10. To run the examples in this repo I had to modify: Line 29 in b095ae5
I did notice that the spinner.succeed/fail/warn methods display as a green v / red x / yellow !! instead of the proper green ✔ / red ✖ / yellow ⚠. These symbols do render properly as text when printed. This appears to be a problem with LogSymbols. |
When using windows terminal, it sets the environment variable "WT_SESSION" this can be used to ensure proper compatibility with windows terminal and non-windows terminal terminals. |
This issue has nothing to do with Windows and everything to do with your terminal emulator. For example, on Windows using Windows Terminal (preview), both CMD and Powershell are able to render the Unicode just fine. The check should be removed. It should be on the user to know what their terminal emulator does and does not support. This check also causes #141. |
resolves manrajgrover#5 resolves manrajgrover#25 resolves manrajgrover#141 obsoletes manrajgrover#142
@norweeg Thanks for looking into this and for the pull request. While I agree that support for UTF-8 has improved on Windows, it is not something that I expect end-user to worry about. As a consumer of the library, I expect to use spinners without worrying about terminal support. For example, if I'm developing a CLI, I expect the library (halo) to check for support before execution. What do you say? |
@manrajgrover I agree with the sentiment, however, the system check also prevents halo from working as expected on Windows for users using Windows terminals which do support UTF-8, of which there are many, as well as in Jupyter Notebooks on Windows. I originally thought halo was broken because it kept ignoring my choice of spinner. I think it would be better not to impose a system limitation on the user, but rather advise Windows users having issues with Unicode in their terminal to either not use a unicode spinner (what you are currently enforcing), use a different terminal, or change their cmd or powershell settings so that they can use a Unicode spinner |
@manrajgrover so I just stumbled into this: https://unix.stackexchange.com/questions/184345/detect-how-much-of-unicode-my-terminal-supports-even-through-screen basically you print a unicode character which is larger than 1 byte to the terminal and poll the location of the cursor. If the cursor doesn't move, or it moves more than 1 column, then your terminal does not support unicode. |
My understanding is that the problem isn't lack of UTF-8 support. It's a lack of fallback font support. Regardless, we could have a user settable variable that enables or disables a check for windows/WT_session (with disabled being the default). Whatever we do. I think it would be far better to give Windows users the full ability to use all the spinners anytime/terminal (rip out the check for windows). Then worry about fallbacks for lesser terminals later. |
@norweeg Thanks for sharing! Yes, it makes sense. Can we look into how other projects handle this issue? Perhaps, have a look at https://github.com/sindresorhus/ora? |
@manrajgrover I'm definitely not an expert in Javascript, but looking over the source code, I'm fairly certain that ora isn't handling the issue at all, forcing all windows users to a fail-safe default line spinner, even if their terminal emulator supports unicode/modern fonts, same as halo does currently. While I understand your desire to have it always look presentable, no matter what terminal the user uses, the check kind of makes halo less appealing on Windows, even when the terminal supports it. That said, the system check needs to be removed from HaloNotebook full-stop. Jupyter, regardless of platform, always supports the advanced spinners because it is handled by the browser. Maybe for terminals make the windows failsafe something like a "force" kwarg that acknowledges that I, the user on Windows, understand that the nicer spinners may not render well in all terminals in windows, but am ok with that knowing that my terminal supports them |
resolves manrajgrover#5 resolves manrajgrover#25 resolves manrajgrover#141 obsoletes manrajgrover#142
resolves manrajgrover#5 resolves manrajgrover#25 resolves manrajgrover#141 obsoletes manrajgrover#142
resolves manrajgrover#5 resolves manrajgrover#25 resolves manrajgrover#141 obsoletes manrajgrover#142
Any progress on this? This works fine on Pycharm on Windows for example, but the is_supported function prevents its use... |
yes, I have been working on it https://github.com/norweeg/halo/tree/remove-terminal-check. The only reason I haven't pull requested it is that I would need to modify some of the tests and I just haven't gotten around to that. My strategy was twofold: for if |
Currently, the package does not support Windows completely. Major reason for that is UTF-8 is not extensively supported by
cmd
,powershell
etc.The text was updated successfully, but these errors were encountered: