8000 Add support to sync Windows Theme with mintty by Klauswk · Pull Request #1305 · mintty/mintty · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add support to sync Windows Theme with mintty #1305

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Add support to sync Windows Theme with mintty #1305

wants to merge 5 commits into from

Conversation

Klauswk
Copy link
@Klauswk Klauswk commented Dec 24, 2024

As discussed in issue #1303, this PR allows Mintty to sync when the user changes the "app mode" in Windows 10/11.

  1. A single new option was added, "Dark theme"
  2. The size of the dropdown "Theme" was changed to match the size of the new dropdown
  3. Add a new option in the config called "dark_theme", with the default value as empty.

image

A few points on my side:

  1. There might be a better place to add the check for the value change, I added it to the src/winmain.c for convenience.
  2. theme_handler wasn't ready to accept more than one dropdown config, I did my best to change the least amount of code as possible, but my test might not have covered all the scenarios
  3. One config being called "theme_file" and the other "dark_theme" might not be the best, suggestions?

@mintty
Copy link
Owner
mintty commented Dec 25, 2024

Thanks for working this out. Initial comments:

I’d keep the position of new attributes in sync in config.h and config.c initialisation, and place dark_theme after theme_file.

Function name set_new_cfg is quite generic, handling only themes.
The handling of either theme attribute could also be supported by two wrapper functions. This would also spare the comparison of label strings, which isn’t done elsewhere I think. Also strcmp with "&Theme" will fail if the UI is localised.
I guess I’ll check details next year...

Initialisation in winmain:
If theme_file is configured, dark mode is ignored.
If dark_theme is configured, its value is ignored.
I guess both is not intended.
Also not sure why you set two specific themes as respective default.
And instead of setting „kohlrausch“, theme should be cleared to the default.

Dynamic adaptation in winmain: I think this shouldn’t be checked continuously but hooked to a suitable Windows message like WM_WININICHANGE.

Style: Please don’t remove empty lines where useful for structuring and keep some space after if, while, etc.

@Klauswk
Copy link
Author
Klauswk commented Dec 25, 2024

Thank you for the review.

  1. Fixed theme config order
  2. Added missing space after if/while
  3. Rollback the new line removal

@mintty
Copy link
Owner
mintty commented Jan 19, 2025

Hi, sorry for the delay.
Did you/Do you intend to continue work on this and provide another PR?
Otherwise, as I cannot accept selected parts of a PR, I would offer this:
I continue to work this out according to my own review comments, then commit it with your authorship, or, if you prefer automatic credits for the contribution, offer the result back to you to submit as a PR.

@Klauswk
Copy link
Author
Klauswk commented Jan 27, 2025

Sorry for the delay as well.

You can go ahead and finish this PR, since some parts might require more knowledge of the system, and right now, unfortunately, I can't put in the time.

As for the strategy, using the authorship seems more reasonable, since both of us would have worked on it.

Thank you for your time.

mintty pushed a commit that referenced this pull request Mar 8, 2025
@mintty
new option ThemeDark to be used if Windows Darkmode is set (#1305, ~#…
59e8c04 
…1303)
mintty added a commit that referenced this pull request Mar 8, 2025
@mintty
adapt theme on change of Windows Dark Mode (#1305, ~#1303)
21ca363
@mintty
Copy link
Owner
mintty commented Mar 8, 2025

I have now integrated the dark mode theme with the following changes:

  • renamed DarkTheme → ThemeDark, to align better with ThemeFile
  • fixed logic to choose darkmode theme in apply_config and main
  • dropped continuous checking in main loop
  • changing theme instead when Windows dark mode changes
  • dropped config in Options dialog for now as some details are still to be considered
    To be checked: interworking with colour scheme configuration (e.g. use of Color Scheme Designer)

@mintty mintty closed this Mar 8, 2025
@mintty
Copy link
Owner
mintty commented Mar 22, 2025

Released 3.7.8 with option ThemeDark, not yet in interactive Options dialog.

@github-actions github-actions bot mentioned this pull request Mar 22, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Klauswk @mintty
0