-
-
Notifications
You must be signed in to change notification settings - Fork 274
TRACKING: Improve platform handling #791
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
Comments
@neiljp for the first part of the limitation, we know if platform.system() is 'Linux':
notify Linux
elif platform.system() is 'Darwin':
notify MacOS This should ensure that notification for the correct platform is always loaded. For the second part I am guessing the value of I am hoping to work on this issue, once the above points are properly discussed. |
The platform should not vary dynamically (ie. while running), so we only need to check once and then look up later - we don't need to keep checking Note that the docs say "such as..." for those values so there may be other values - and this may not always be the best way of identifying the platforms, in the way we do it differently for WSL right now. In any case the point is to hide and centralize this in the new wrapper file so we just request an action from it, rather than looking-up platforms in each location. Once we have platforms encoded, we can also provide string representations and feature support feedback. For example, what is the current platform identified as? Is the current platform fully supported? What is/is-not supported? This could go into the About popup. |
This commit adapts our codebase to use the new PLATFORM variable instead of the 3 booleans that stood for each of the supported platforms i.e. WSL, MACOS and LINUX. This ensures that only one platform (OS) is set for each instance. We modify the platform-dependent codes in various modules to use this new variable, which prevents bugs that arised earlier due to multiple OS'es being detected in WSL. Discussion about the bug and how it affected some features in WSL: #zulip-terminal>Platform handling (#T791) and WSL issues. Tests amended. Fixes zulip#791.
This commit adapts our codebase to use the new PLATFORM variable instead of the 3 booleans that stood for each of the supported platforms i.e. WSL, MACOS and LINUX. This ensures that only one platform (OS) is set for each instance i.e. it is mutually exclusive. We modify the platform-dependent codes in various modules to use this new variable, which prevents bugs that arised earlier due to multiple OS'es being detected in WSL. Discussion about the bug and how it affected some features in WSL: #zulip-terminal>Platform handling (#T791) and WSL issues. Tests amended. Fixes zulip#791.
This commit adapts our codebase to use the new PLATFORM variable instead of the 3 booleans that stood for each of the supported platforms i.e. WSL, MACOS and LINUX. This ensures that only one platform (OS) is set for each instance i.e. it is mutually exclusive. We modify the platform-dependent codes in various modules to use this new variable, which prevents bugs that arised earlier due to multiple OS'es being detected in WSL. Discussion about the bug and how it affected some features in WSL: #zulip-terminal>Platform handling (#T791) and WSL issues. Tests amended. Fixes zulip#791.
This commit adapts the codebase to use the new PLATFORM variable instead of the 3 booleans that stood for each of the supported platforms i.e. WSL, MACOS and LINUX. This ensures that only one platform (OS) is set for each instance i.e. it is mutually exclusive. We modify the platform-dependent codes in various modules to use this new variable, which prevents bugs that arose earlier due to multiple OS'es being detected in WSL. Discussion about the bug and how it affected some features in WSL: #zulip-terminal>Platform handling (#T791) and WSL issues. Tests amended. This is a partial fix towards zulip#791.
This commit adapts the codebase to use the new PLATFORM variable instead of the 3 booleans that stood for each of the supported platforms i.e. WSL, MACOS and LINUX. This ensures that only one platform (OS) is set for each instance i.e. it is mutually exclusive. We modify the platform-dependent codes in various modules to use this new variable, which prevents bugs that arose earlier due to multiple OS'es being detected in WSL. Discussion about the bug and how it affected some features in WSL: #zulip-terminal>Platform handling (#T791) and WSL issues. Tests amended. This is a partial fix towards #791.
I just split the UI part out into #1216 making that more accessible, leaving this remaining as infrastructure only. |
Currently we use
platform
to determine which platform we're running on, currently primarily for notifications.There are currently some limitations in this:
The former means - admittedly in likely rare situations - that we may load multiple/incorrect notifications. Multiple platforms should be an error that either the application or user disambiguates.
The latter means that other platforms, such as likely BSD or more broadly POSIX-like systems may not run properly, or at minimum will eg just transparently not show notifications as it stands.
We currently have the platform detection and notification code in
helper.py
; I'd suggest we extract this code and have platform-dependent code be isolated in a separate module, with other code not being otherwise aware of any underlying platform, just whether things are working/available or not.edit
Expected functionality to complete this:
The text was updated successfully, but these errors were encountered: