bug-gnu-emacs
[Top][All Lists]
Advanced

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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]