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.

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

Closed
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
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.

qdzhaov added a commit to qdzhaov/mintty that referenced this pull request Jul 5, 2025
merge from f18e7a4
commit f18e7a4  Wed Jun 4 2025
    avoid repetitive painting of images
commit c6998b4 Sat May 24 2025
    change default setting UnderlineManual=true for consistent line placement
commit 546bae7 Sun May 18 2025
    accept (but ignore) XTMODKEYS subparameters (xterm 398)
commit 92d37ab Tue Apr 1 2025
    fix emoji invisible and blinking attributes
commit 9a5ac91 Tue Apr 1 2025
    fix darkmode/wakeup refresh to not override dynamic OSC colour settings
commit 6d2730d Mon Mar 24 2025
    wiki Tips: workaround for mouse mode interaction with console-based programs (mintty#1319)
commit c1db151 (tag: 3.7.8) Sat Mar 22 2025
    3.7.8
commit b1ea03f Sat Mar 22 2025
    tweak Makefile
commit c5d045b Fri Mar 21 2025
    tweak handling of image size limit
commit 34ea393 Fri Mar 21 2025
    change image size limit to configurable value,
    new option MaxImageSize, default 4444444
commit 32fc955 Fri Mar 21 2025
    fix reflow to avoid vanishing graphics on terminal resizing
commit a2ee38f Wed Mar 19 2025
    hair cross mouse pointer in pixel-grained mouse reporting modes;
    new option PixMousePointer
commit 0b50584 Mon Mar 17 2025
    fix frame adjustment when disabling darkmode
commit 00b9b81 Sun Mar 16 2025
    fix mouse position click coordinates to adapt to horizontal scrolling
commit 28673e0 Sun Mar 16 2025
    fix graphics (image/sixel) and emoji display to adapt to horizontal scrolling
commit db391ac Sun Mar 9 2025
    Arabic joining considers ZWJ and ZWNJ formatters
commit b784671 Sun Mar 9 2025
    minibidi code cleanup
commit 21ca363 Sat Mar 8 2025
    adapt theme on change of Windows Dark Mode (mintty#1305, ~mintty#1303)
commit 59e8c04 Sat Mar 8 2025
    new option ThemeDark to be used if Windows Darkmode is set (mintty#1305, ~mintty#1303)
commit 4c73970 Tue Mar 4 2025
    changed prefix ">" in option FontChoice to center CJK ranges (mintty#1313)
commit b819ad0 Sat Mar 1 2025
    man page: proper rendering of ^
commit 8499fd4 Tue Feb 25 2025
    propagate TERM config setting to WSL HOSTTERM variable (mintty/wsltty#290, ~mintty/wsltty#278)
commit e19baf9 Tue Feb 25 2025
    wiki Tips: mention “emoji width” mode (DECSET 2027)
commit f968dd2 Sun Feb 23 2025
    wiki Tips: advice to the conhost patch to make a backup copy first (~mintty#1239)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants