[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#51404: Support system dark mode on Windows 10
From: |
Eli Zaretskii |
Subject: |
bug#51404: Support system dark mode on Windows 10 |
Date: |
Sat, 29 Jan 2022 10:40:01 +0200 |
> From: Vince Salvino <salvino@coderedcorp.com>
> CC: Eli Zaretskii <eliz@gnu.org>
> Date: Sat, 29 Jan 2022 03:34:32 +0000
>
> Update: I improved the previous patch to use a linked list to track the
> window handles during runtime, and am reasonably happy with it. If this looks
> good please go ahead and install the attached patch 0002 to master. Thanks!
Thanks. A few comments below, mostly about minor stylistic issues.
> From a8c2f353372d8f015538804e17682e72e40af222 Mon Sep 17 00:00:00 2001
> From: Vince Salvino <salvino@coderedcorp.com>
> Date: Fri, 28 Jan 2022 22:25:13 -0500
> Subject: [PATCH] Support MS-Windows light/dark mode theme change during
> runtime. (Bug#51404)
Please provide a ChangeLog-style description of changes (see
CONTRIBUTE for the details of the format we prefer).
> -/* If the OS is set to use dark mode. */
> +/* If the OS supports light/dark mode. */
^^
Our style is to leave 2 spaces after the final period of the comment
(here and elsewhere in your patch).
> +/* Simple linked list to track window handles during runtime so they
> + can be updated if the Windows light/dark mode theme is changed. */
> +struct HWND_NODE
> +{
> + HWND hwnd;
> + struct HWND_NODE *next;
> +};
> +struct HWND_NODE *g_hwnd_root;
I see where you add windows to the list, but I don't see where you
remove deleted windows from the list. Does that mean the list will
always grow indefinitely through an Emacs session, even if windows are
deleted?
> /* Applies the Windows system theme (light or dark) to the window
> - handle HWND. */
> + handle HWND. `track` should generally be TRUE to keep a reference
^^
Two spaces between sentences there. Also, our style of quoting in
comments is 'like this', not MD-style `like this`.
> + /* After applying the theme, add the HWND to our global list so
> + it can be changed later if the OS light/dark mode theme is
> + changed. */
> + if(track)
> + {
> + struct HWND_NODE *curr = malloc(sizeof(struct HWND_NODE));
Please use xmalloc, not malloc, to allocate memory.
Also, our style is to leave one space between a function name and the
opening parenthesis that follows it.
> + /* Loop through all known HWNDs and apply theme */
^^
Each comment should end with a period (and 2 spaces).
> + struct HWND_NODE *curr = g_hwnd_root;
> + while ( curr != NULL )
^^^^^^^^^^^^^^
Our style is NOT to leave a space after the opening parenthesis and
before the closing parenthesis.