Feature windows support (WIP) #292
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Most functionality does work at this point. It can: - Download and change a background from many providers - Most options seem to work fine - RPC calls work OK for client as well with asyncio - System tray icon behaves itself a little better but does need to be clicked twice to take away the menu dialogue (seems like a known issue with gtk / StatusIcon on windows) But: - It has not been tested on linux yet to verify these changes haven't broken anything - I need to set it up with a clean build because looks like msys have reorganized things since the chocolatey build I was using. This is a must for packaging and CI - There is a deadlock / crash when quickly selecting checkboxes (I see a recent commit may fix that - need to merge and test) - History and Wallpaper Selector seems broken - Implementation for IPC using asyncio and unix socket is in there for when it is finally supported for python on windows, but is untested on linux as well. - Quotes don't work on windows nor the digital clock. Probably never will so may want to remove associated menu items on windows. In fact nothing from Effects tab seems to work. You need some MSYS things - I'm in the process of trying to figure out exactly what, but it is not trivial and seems to be subject to change. One of the fixes involved having to copy a system library do a different name for lxml.
jlu5
reviewed
Feb 22, 2020
| self.server = self.service.loop.run_until_complete(self.coro) | ||
| # Serve requests until Ctrl+C is pressed | ||
| try: | ||
| self.service.loop.run_until_complete(self.server.wait_closed()) |
There was a problem hiding this comment.
FWIW I'm not sure how safe it is to mix threads and asyncio code. It might beeasier to just stick to one concurrency concept at a time?
jlu5
reviewed
Feb 22, 2020
| elif isinstance(value, int): | ||
| dtype = winreg.REG_DWORD | ||
| else: | ||
| dtype = winreg.REG_SZ |
There was a problem hiding this comment.
nit: we can probably just collapse this into one check for isinstance(value, int)?
jlu5
reviewed
Feb 22, 2020
| elif master == 'HKEY_LOCAL_MACHINE': | ||
| registry = winreg.HKEY_LOCAL_MACHINE | ||
| elif master == 'HKEY_USERS': | ||
| registry = winreg.HKEY_USERS |
There was a problem hiding this comment.
If no hives other than HKEY_CURRENT_USER are used we can probably remove those cases. AFAICT HKEY_LOCAL_MACHINE shouldn't be writable from an unprivileged account anyways.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi Folks,
Raising initial PR for #44. It seems to at least not break linux (any more) and should make the code portable for windows. Raising this to get the code reviewed at this point while I figure out how best to pack this up.
Cheers,
Frank