GNU bug report logs - #51404
Support system dark mode on Windows 10

Previous Next

Full log

Message #69 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vince Salvino <salvino <at> coderedcorp.com>
Cc: 51404 <at> debbugs.gnu.org
Subject: Re: Support system dark mode on Windows 10
Date: Sat, 29 Jan 2022 10:40:01 +0200

> From: Vince Salvino <salvino <at> coderedcorp.com>
> CC: Eli Zaretskii <eliz <at> 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 <at> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.