zulip-terminal
zulip-terminal copied to clipboard
TRACKING: Improve platform handling
Currently we use platform to determine which platform we're running on, currently primarily for notifications.
There are currently some limitations in this:
- while not expected, we don't ensure that these values are mutually exclusive (only one is set)
- we explicitly only check for (and test on) these platforms
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:
- [X] Ensure mutually exclusive detection (#1059)
- [x] Isolate platform-specific code into file (mostly via #1059 ?)
- [x] Output detected platform on startup and in About menu ** (see #1216)
- [ ] Relate platform detection to platforms we're running CI on (ensure we're running where we think; potentially expand CI?)
- [ ] If possible, ensure functionality that is platform-specific is actually tested on those platforms (only)
@neiljp for the first part of the limitation, we know platform.system() will always be one of 'Linux', 'Darwin', 'Java' or 'Windows', so instead of pre-checking for the correct platform values(which may not be mutually exclusive), we can check for the system on the go inside notify function. Something like this:
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 platform.system() will be empty, if the platform is BSD or other POSIX-based. So, I am not sure how to handle this limitation. But we can surely discuss this further.
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 platform.system(). We can do this via the new platform-agnostic wrapper file.
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.
I just split the UI part out into #1216 making that more accessible, leaving this remaining as infrastructure only.