Add system theme support #806

zwimer · 2022-12-01T02:11:01Z

Implements: #780
Fixes: #805

github-actions · 2022-12-01T02:26:58Z

Unit Test Results

1 tests - 7 1 ✔️ - 7 0s ⏱️ - 1m 0s
1 suites - 7 0 💤 ±0
1 files - 7 0 ❌ ±0

Results for commit d2474ae. ± Comparison against base commit c2d5747.

This pull request removes 7 tests.

test_qaddress_input.TestQaddressInput ‑ test_address_conversion
test_qaddress_input.TestQaddressInput ‑ test_function_name
test_rename_functions.TestRenameFunctions ‑ test_rename_a_callee_in_pseudocode_view
test_rename_functions.TestRenameFunctions ‑ test_rename_a_function_in_disasm_and_pseudocode_views
test_rename_variables.TestRenameVariables ‑ test_rename_a_global_variable_in_pseudocode_view
test_rename_variables.TestRenameVariables ‑ test_rename_a_local_variable_in_pseudocode_view
test_workflow.TestWorkflow ‑ test_workflow

♻️ This comment has been updated with latest results.

zwimer · 2022-12-02T07:26:04Z

A better way to do this would be to add an observer with a callback, rather than polling. I'll investigate this.

A possible example: https://github.com/mborgerson/TrayPlay/blob/5a9e43ca4c32283f6253761b69858ba43ee48720/TrayPlay/AppDelegate.m#L52

zwimer · 2022-12-06T02:03:35Z

I made a PR on dark detect to add a listener for macoOS: albertosottile/darkdetect#30

Once it is merged and updated on pypi, I can update the PR to use the listeners instead of doing polling.

zwimer · 2022-12-08T07:33:42Z

In order to avoid leaving potentially orphaned subprocesses around, I've filed this issue: albertosottile/darkdetect#31

zwimer · 2022-12-08T23:07:16Z

Once albertosottile/darkdetect#32 is merged I'll update this PR; hopefully a new PyPi release will be out soon at that point.

zwimer · 2023-01-19T23:25:04Z

CI will continue to fail until the darkdetect PR is merged and a new pypi release is made.

for more information, see https://pre-commit.ci

github-actions · 2023-04-25T22:57:56Z

Test Results

19 tests ±0 19 ✔️ ±0 1m 54s ⏱️ +20s
10 suites ±0   0 💤 ±0
10 files ±0   0 ❌ ±0

Results for commit 1b1092f. ± Comparison against base commit 0b37a4b.

♻️ This comment has been updated with latest results.

… in config

zwimer · 2023-04-26T02:43:20Z

The test failures are also failing on master.

zwimer · 2023-04-26T02:43:58Z

Upon merging, #780 and #805 should both be marked as complete.

angrmanagement/ui/dialogs/preferences.py

mborgerson · 2023-04-26T19:29:05Z

angrmanagement/ui/dialogs/preferences.py

@@ -235,6 +259,9 @@ def save_config(self):
        for i in self._font_options:
            i.update()

+    def revert_unsaved(self):


Identical to base class implemantation

The base class raises NotImpelementedError.

Oh, I guess I was looking at the wrong thing

Why not have the default implementation do nothing instead of raising an error? Or just drop the method because there's not an implementation that does anything?

I was generally thinking of the page class as abstract and the functions that raise NotImpelemented as 'things derived classes should implement' (even if they are just no-op's). It's just a different paradigm but either way should be equivalent in this context. I don't have strong preferences either way so I can nuke the function if you prefer.

angrmanagement/ui/dialogs/preferences.py

angrmanagement/utils/track_system_theme.py

mborgerson · 2023-04-26T19:53:22Z

setup.cfg

@@ -29,6 +29,7 @@ install_requires =
    qtterm
    requests[socks]
    tomlkit
+    darkdetect-angr[macos-listener]


It's unfortunate that we have to maintain a forked release of darkdetect now, but I appreicate the work on it

Did Windows/Linux get tested?

The majority of the fork has a PR into the main repo, so once that is merged we might not need to hopefully. TBD. I've tested the PR'd version on all three OS's, though less throughly on Windows as I lack said OS. The version we are using is mostly the same so it ought to work. Feel free to test them yourself but the relevant logic that distinguished between OSes wasn't really the code that changed between the PR'd version and the hardfork version.

I've tested the PR'd version on all three OS's

But not the forked version with extra changes?

Feel free to test them yourself

I don't plan to test this PR. Will you please test it

The extra changes are copyright, README, and __version__, and pyproject.toml to change the package name. Not really the code.

If you are curious, here is the diff stats:

LICENSE | 4 ++-- README.md | 8 ++++---- darkdetect/__init__.py | 4 ++-- darkdetect/__main__.py | 2 +- darkdetect/_dummy.py | 2 +- darkdetect/_linux_detect.py | 2 +- docs/api.md | 2 +- docs/examples.md | 2 +- pyproject.toml | 10 +++++-----

Note the 2 line changes are copyright comments. I don't have access to a Windows device at the moment so I can't really test these changes easily, but they aren't really code changes just the changes necessary to hard-fork a project (changing the name on pypi, copyright, documentation, and version).

I do not know how to automate GUI test cases on windows changing system theme and detecting of the colors properly all changed over. Feel free. The actions to test this would be launch app, open preferences, select 'Track System Theme', then toggle the Windows System Theme and visually ensure it tracks the theme.

You don't need to "visually ensure" it, you just need to make sure after the theme is changed the correct behavior occurs in the library. I'm sure applescript and powershell each have ways to adjust the system theme.

If you want to write these test cases, I can tell you how to do it in applescript (via System Events), I don't know powershell. I imagine for Gnome specifically there is a gsettings option you could find? For detecting if it worked, you'll have to query the QApplication to check that all the colors and such have been updated and also ensure that any visual refresh functions have been called (i.e. redrawing the GUI app).

That's on the angr management side, but not necessary for the library itself which is is the subject of this thread.

I'll happily take a PR for them here: https://github.com/zwimer/darkdetect but I don't have plans to write tests myself for them at the moment. If you do PR them, I'll also merge them into the branch that is PR-ing into the original repo https://github.com/albertosottile/darkdetect so as to avoid actively diverging the hard fork from the original more than necessary; my plan is to get rid of the fork once the PR albertosottile/darkdetect#32 in the original is merged.

Right now that PR has been tested on all three OSes and my fork's master branch doesn't meaningfully diverge from said code. I know the maintainer desired to do more thorough testing before merging the PR though, so I'm sure the test cases would be appreciated by all parties.

Add system theme support

49ddefe

zwimer linked an issue Dec 1, 2022 that may be closed by this pull request

Allow theme based on system preferences #780

Open

zwimer self-assigned this Dec 1, 2022

zwimer added 2 commits November 30, 2022 21:20

Bug fixes

6a05e7b

Respect save button

7dfc3ca

zwimer marked this pull request as ready for review December 1, 2022 04:30

zwimer requested a review from mborgerson December 1, 2022 04:31

zwimer mentioned this pull request Dec 1, 2022

Allow theme based on system #807

Closed

lint

c43c420

zwimer marked this pull request as draft December 2, 2022 07:26

zwimer mentioned this pull request Dec 2, 2022

No 'current' theme #805

Open

Merge branch 'master' into 780-allow-theme-based-on-system-preferences

8e6a4e9

zwimer added 3 commits January 17, 2023 17:23

merge master

00c8d96

Update to use upcoming darkdetect API

7ebefd6

Bug fixes

d2474ae

zwimer and others added 3 commits April 25, 2023 15:43

merge master

5dcac4b

Use darkdetect-angr

ce422a3

[pre-commit.ci] auto fixes from pre-commit.com hooks

0811d4b

for more information, see https://pre-commit.ci

zwimer added 2 commits April 25, 2023 16:39

Bug fix in instance that format string with duplicate output is saved…

5fc4cdd

… in config

Lint

5ee69bb

zwimer marked this pull request as ready for review April 26, 2023 02:43

mborgerson reviewed Apr 26, 2023

View reviewed changes

Move init to top of class

1b1092f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add system theme support #806

Add system theme support #806

zwimer commented Dec 1, 2022 •

edited

Loading

github-actions bot commented Dec 1, 2022 •

edited

Loading

zwimer commented Dec 2, 2022 •

edited

Loading

zwimer commented Dec 6, 2022

zwimer commented Dec 8, 2022

zwimer commented Dec 8, 2022

zwimer commented Jan 19, 2023

github-actions bot commented Apr 25, 2023 •

edited

Loading

zwimer commented Apr 26, 2023

zwimer commented Apr 26, 2023

mborgerson Apr 26, 2023

zwimer Apr 26, 2023

mborgerson Apr 26, 2023

zwimer Apr 26, 2023

mborgerson Apr 26, 2023

zwimer Apr 26, 2023

mborgerson Apr 26, 2023

zwimer Apr 26, 2023

zwimer Apr 26, 2023

zwimer Apr 26, 2023

twizmwazin Apr 26, 2023

zwimer Apr 26, 2023

twizmwazin Apr 26, 2023

zwimer Apr 26, 2023

Add system theme support #806

Are you sure you want to change the base?

Add system theme support #806

Conversation

zwimer commented Dec 1, 2022 • edited Loading

github-actions bot commented Dec 1, 2022 • edited Loading

Unit Test Results

zwimer commented Dec 2, 2022 • edited Loading

zwimer commented Dec 6, 2022

zwimer commented Dec 8, 2022

zwimer commented Dec 8, 2022

zwimer commented Jan 19, 2023

github-actions bot commented Apr 25, 2023 • edited Loading

Test Results

zwimer commented Apr 26, 2023

zwimer commented Apr 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zwimer commented Dec 1, 2022 •

edited

Loading

github-actions bot commented Dec 1, 2022 •

edited

Loading

zwimer commented Dec 2, 2022 •

edited

Loading

github-actions bot commented Apr 25, 2023 •

edited

Loading