GNU bug report logs - #79859
Add dock-mode for system GUI dock icon support

Previous Next

To reply to this bug, email your comments to 79859 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

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

From: Stéphane Marks <shipmints <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Cc: Björn Bidar <bjorn.bidar <at> thaodan.de>
Subject: Add dock-mode for system GUI dock icon support
Date: Tue, 18 Nov 2025 14:17:45 -0500

[Message part 1 (text/plain, inline)]

This patch implements system GUI dock features for icon badges, icon
progress indicator, and icon attention request with implementations for
GNU/Linux KDE & GNOME, NS/macOS, and MS-Windows.

GNU/Linux shells need to use an extension or include built-in support for
the protocol documented here https://wiki.ubuntu.com/Unity/LauncherAPI.  I
tested using this "standard" shell extension
https://extensions.gnome.org/extension/307/dash-to-dock/ on GNOME and
assistance and testing on KDE from the reddit user GeneAutomatic3471.

I'd appreciate thoughtful feedback, and of course, testing.

-Stéphane

[Message part 2 (text/html, inline)]

[0001-Add-system-GUI-dock-integration.patch (application/octet-stream, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stéphane Marks <shipmints <at> gmail.com>
Cc: bjorn.bidar <at> thaodan.de, 79859 <at> debbugs.gnu.org
Subject: Re: bug#79859: Add dock-mode for system GUI dock icon support
Date: Thu, 20 Nov 2025 12:27:49 +0200

> Cc: Björn Bidar <bjorn.bidar <at> thaodan.de>
> From: Stéphane Marks <shipmints <at> gmail.com>
> Date: Tue, 18 Nov 2025 14:17:45 -0500
> 
> This patch implements system GUI dock features for icon badges, icon progress indicator, and icon attention
> request with implementations for GNU/Linux KDE & GNOME, NS/macOS, and MS-Windows.

Thank you for working on this important feature.

> GNU/Linux shells need to use an extension or include built-in support for the protocol documented here
> https://wiki.ubuntu.com/Unity/LauncherAPI.  I tested using this "standard" shell extension
> https://extensions.gnome.org/extension/307/dash-to-dock/ on GNOME and assistance and testing on KDE
> from the reddit user GeneAutomatic3471.

This information should be in the NEWS entry.

> I'd appreciate thoughtful feedback, and of course, testing.

I wanted to test this on MS-Windows, but the w32fns.c patch failed to
apply to the master branch, even if I use --ignore-whitespace option
to "git apply".  Could you please rebase on the current master and
post an updated patch?

So the below comments are based on reading the patch and nothing else.

> From f1c4bbf07a4b1c0cb5867412ecf2074bfc3e37e1 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?St=C3=A9phane=20Marks?= <shipmints <at> gmail.com>
> Date: Sat, 8 Nov 2025 13:30:37 -0500
> Subject: [PATCH] Add system GUI dock integration.
> 
> Implement system GUI dock icon badge, icon progress indicator,
> icon attention alert features for D-Bus platforms (tested on KDE
> and GNOME), NS (GNUstep/macOS), Microsoft Windows.
> 
> * lisp/dock-mode.el: New file.

First, the name of the mode and related symbols: I find the name
"dock" to be problematic, as it seems to be well-known to macOS users
and maybe also to some users of GNU/Linux desktops, but might be
unknown to others.  Also, "dock" has multiple meanings, which might be
confusing.  I would prefer something like dock-taskbar-* instead, or
maybe gui-dock-*, or even gui-desktop-* (if we envision more features
being added to this in the future).  I'm also open to other ideas
about better naming this class of features.

> diff --git a/doc/misc/dock-mode.texi b/doc/misc/dock-mode.texi
> new file mode 100644
> index 00000000000..71358c09b7e
> --- /dev/null
> +++ b/doc/misc/dock-mode.texi

I don't think this should be a separate Info file.  Instead, we should
split this information into a user-level part and Lisp-programmer
level part, and have the former in the user manual and the latter in
the ELisp reference.  Most of the Texinfo documentation you wrote
naturally belongs to the latter category.

A separate Info manual has a significant disadvantage that it is not
part of the two main manuals, and is thus harder to discover.

> +@node Overview
> +@chapter Overview
> +
> +The Dock package provides integration with these system GUI dock
> +features:

This should start by explaining what is a "GUI dock".  It is an
obscure term that is unlikely to be widely known, IME.

> +Your GNU/Linux Emacs instance should be launched via an appropriate
> +shell "desktop" file such as those distributed with Emacs; e.g.,
> +"etc/emacsclient.desktop".

File names should have the @file markup, instead of quoting them (here
and elsewhere in the patch).

> +@node User Options
> +@chapter User Options

This node should go to the Emacs user manual.  But it should begin by
describing the features in user-facing terms: what will the user see
for each aspect of the mode: how badges look, what is "attention
indicator", etc.  This stuff is unlikely to be well-known to users,
and without that, the following description of user options makes very
little sense.

> +@defvar dock-dbus-desktop-file-name
> +The variable @code{dock-dbus-desktop-file-name} applies on GNU/Linux
> +D-Bus systems, and can be customized to indicate which desktop file name
> +D-Bus will use to identify your Emacs instance.  It defaults to the
> +desktop file base name @code{"emacsclient"} and might need to be set to
> +@code{"emacs"} on your system.  Installed Emacs builds typically include
> +default desktop files for both, and are called
> +@code{"emacsclient.desktop"} and @code{"emacs.desktop"}, respectively.
> +@end defvar

Do users frequently need to customize this?  If in most cases this
doesn't need to be customized, we don't need to mention the option in
the manual.

> +@defvar dock-dbus-timeout-ms
> +The variable @code{dock-dbus-timeout-ms} applies on GNU/Linux D-Bus
> +systems, and can be customized to specify the number of milliseconds
> +Emacs will wait for D-Bus responses before timing out and considering
> +messages undeliverable.  It defaults to 25000 milliseconds, which is the
> +recommended default by the D-Bus package, which see, @pxref{Top,,, dbus,
> +D-Bus integration in Emacs}.

This doesn't seem to be a user-level option, at least not an important
one.  I presume that the default is fine for almost all uses, and the
option only needs customization when troubleshooting.

> +@node Function Reference
> +@chapter Function Reference
> +
> +@findex dock-mode
> +@defun dock-mode
> +@code{dock-mode} must be enabled before calling any of its functions, to
> +initialize the system back end.  On GNU/Linux D-Bus, initialization will
> +test that the D-Bus dock protocol is established, and signal an error if
> +it is not.
> +@end defun

This mode, and the description of GUI features it activates, should in
the user manual.

> +@findex dock-badge
> +@defun dock-badge &optional count-or-string
> +Set the dock icon badge to @var{count-or-string}.  If
> +@var{count-or-string} is @code{nil}, clear the badge.

I'm missing here the description of where will the badge appear if
Emacs has several visible frames.  Will the badge appear on all of
them? only one? something else?

> +If a string is specified on back ends which do not support strings, it
> +is converted to an integer, if possible, or to nil.  GNUstep and macOS
> +support string badges, the GNU/Linux D-Bus protocol supports only
> +integers; any string that cannot be converted to integer will be treated
> +as @code{nil}.

This describes the implementation, but says nothing about the effect.
What would be displayed when a string is converted to an integer (and
how can an arbitrary string be converted to a number anyway)?
Depending on the answers to those questions, it is possible that we
don't have to mention the specific systems and backends.

> +@findex dock-attention
> +@defun dock-attention &optional urgency timeout

There's no need to have @findex for a @defun (here and elsewhere in
the patch), because @defun automatically generates an index entry in
the function index.

> +Set the dock icon to alert the user.  This will result in a bouncing or
> +flashing dock icon.  On some back ends, dock icon attention will
> +displayed only if Emacs is not the focused application.     ^^^^
   ^^^^^^^^^
"will be displayed"

> +out automatically.  In contrast, a `critical` request requires that the

Please don't quote `like this` in Texinfo documents, please use @code
or @samp instead.

> +@defun dock-progress &optional progress
> +Set the dock icon to indicate ``progress''.  Progress is typically
> +represented as a graphical bar that fractionally spans the dock icon.

This should have a cross-reference to the node "Progress" in the ELisp
manual, which describes progress indicators.  It is also a good idea
to have the cross-reference in "Progress" pointing to this
description.

> +icon.  Its usage is described in @pxref{Progress,, Display, elisp, The
> +Emacs Lisp Reference Manual}.

This kind of "HTML-style" cross-references doesn't look good in Info
and in the printed versions of the manual.  It is better to use
something simpler, like

  This is a convenience macro analogous to
  @code{dotimes-with-progress-reporter} and works the same way as
  @code{dotimes} does, but also reports loop progress on the Emacs dock
  icon.  @xref{Progress,, Display, elisp, The Emacs Lisp Reference Manual}.

(Btw, that "Display" part in the cross-reference is wrong, and will
not do what you thought it would.)

> +@defmac dolist-with-dock-progress-reporter (var list [result]) reporter-or-message body <at> dots{}
> +This is a convenience macro analogous to
> +@code{dolist-with-progress-reporter} and works the same way as
> +@code{dolist} does, but also reports loop progress on the Emacs dock
> +icon.  Its usage is described in @pxref{Progress,, Display, elisp, The
> +Emacs Lisp Reference Manual}.

Same comments here.

> +@lisp
> +;; Enable and initialize dock-mode.
> +(dock-mode)
> +
> +;; Display a badge on the dock icon.
> +(dock-badge emacs-major-version)
> +(dock-badge "31")
> +(dock-badge "Test") ; short string (on supported platforms)

Such long @lisp snippets should be subdivided into groups, using
@group..@end group, to prevent the group from being split between two
pages, which makes reading the examples harder.

> +        ((bound-and-true-p w32-initialized)
> +         (setq dock--back-end 'windows-nt))
                                 ^^^^^^^^^^^
I'd prefer the symbol 'w32' here.

> +(define-minor-mode dock-mode
> +  "GUI dock icon badges, progress report, alerts."

This doc string should be significantly expanded to describe the mode
features, with links to relevant commands and user options.

Also, please add a :version tag.

> +(cl-defgeneric dock-badge (&optional count-or-string)
> +  "Set the dock icon badge to COUNT-OR-STRING.
> +If COUNT-OR-STRING is an integer, display that as the icon badge.
> +If COUNT-OR-STRING is a string on back ends which do not support
> +strings, convert COUNT-OR-STRING to an integer, or nil.
> +If COUNT-OR-STRING is nil, or is an empty string, remove the counter.")

If a string is converted to nil, will that remove the counter?

> +(cl-defgeneric dock-attention (&optional urgency timeout)
> +  "Set the dock icon to alert the user.

I think "Display the dock icon to alert the user." is a better
description.

And I wonder whether we should, under dock-mode, call dock-attention
as part of bell and visible-bell, at least as a user option?

> +(cl-defgeneric dock-progress (&optional progress)
> +  "Set the dock icon to indicate progress.

Likewise.

> +(defvar dock--dotimes-with-dock-progress-reporter-advice-count 0
> +  "Ensure advice is not removed when more than one active use.")
> +
> +(defun dock--set-up-progress-reporter ()
> +  "`progress-reporter-do-update` helper."
> +  (incf dock--dotimes-with-dock-progress-reporter-advice-count)
> +  (advice-add #'progress-reporter-do-update
> +              :around
> +              #'dock--progress-reporter-do-update))

Our conventions frown on using advice in our code.  How about adding a
hook to progress-reporter-do-update instead, so that it could call
this indication of progress when the mode is active?

> +(defun dock--tear-down-progress-reporter ()
> +  "`progress-reporter-do-update` helper."
> +  (when (= 0 (decf dock--dotimes-with-dock-progress-reporter-advice-count))
> +    (advice-remove #'progress-reporter-do-update
> +                   #'dock--progress-reporter-do-update)))

Same here.

> +(defvar dock-w32-badge-bg '(#xe7 #x58 #x57) ; redish
> +  "Integer RGB values as the background for w32 badge.")
> +
> +(defvar dock-w32-badge-fg '(#xff #xff #xff) ; white
> +  "Integer RGB values as the foreground for w32 badge text.")

Can't we use the normal#RRRGGGBBB color notations here? why invent
something new when we already have infrastructure for specifying
colors flexibly and easily?

> diff --git a/src/w32fns.c b/src/w32fns.c
> index 3fc0f55244f..c0d7cad3dc5 100644
> --- a/src/w32fns.c
> +++ b/src/w32fns.c

As mentioned above, I was unable to compile the code, so the comments
below are based only on reading the patch.

> @@ -38,6 +38,10 @@ #define COBJMACROS /* Ask for C definitions for COM.  */
>  #include <shlobj.h>
>  #include <oleidl.h>
>  #include <objidl.h>
> +#if !defined MINGW_W64 && !defined CYGWIN
> +# define INITGUID
> +#endif

I don't understand this: why only MINGW_W64? does this code work only
in 64bit builds or something?

And why define INITGUID to nothing?

> +#ifndef IID_ITaskbarList3
> +DEFINE_GUID(IID_ITaskbarList3, 0xea1afb91, 0x9e28, 0x4b86, 0x90,0xe9, 0x9e,0x9f,0x8a,0x5e,0xef,0xaf);

Please add a comment here explaining what these constants are and what
is their effect.  Or maybe these constants are required by Windows to
be that and only that?  This should be documented, preferably with a
reference.

> +/* Parse RGB_LIST, and return a COLORREF.
> +   RGB_LIST should contain (at least) 3 lisp integers.
> +   Return 0xff000000 iff RGB_LIST is not OK.  */
> +
> +static COLORREF
> +rgb_list_to_colorref (Lisp_Object rgb_list)

I believe this function will be unnecessary if you use the usual
#RRRGGGBBB color spec instead of a list of RGB components.

> +/* Track the window handle for the global badge icon. */
> +static HWND _w32_badge_hwnd = NULL;

Please don't use variables with leading underscores, that's only
appropriate for libraries.

Also, does this mean we can only have a single badge on display at any
given time?  And what happens if the frame corresponding to this
handle is deleted?

> +DEFUN ("w32-badge",
> +       Fw32_badge,
> +       Sw32_badge,
> +       3, 3, 0,
> +       doc: /* Set the taskbar icon overlay image based on BADGE.
> +BADGE is a UTF-8 string.  If BADGE is nil, remove the overlay.  Do

Please say "BADGE is a string", without mentioning UTF-8.  Callers of
primitives should pass Emacs Lisp strings, and the primitive should
then encode the string as needed (which your code already does).

> +                                 Do nothing if the frame is not (yet)
> +associated with an HWND.

Which frame?  And there's no argument named HWND.

>                           BACKGROUND and FOREGROUND are lists of (RED
> +GREEN BLUE) 8-bit integer values.  */)
> +  (Lisp_Object badge, Lisp_Object bg, Lisp_Object fg)

The color arguments are named BG and FG, not BACKGROUND and
FOREGROUND.

> +{
> +  CoInitialize (NULL);
> +  ITaskbarList3 *task_bar_list = NULL;
> +  HRESULT r = CoCreateInstance(&CLSID_TaskbarList,
> +			       NULL,
> +			       CLSCTX_INPROC_SERVER,
> +			       &IID_ITaskbarList3,
> +			       (void **)&task_bar_list);
> +  if (r != S_OK)
> +    {
> +      /* FIXME: Do we need to call CoUninitialize?  */

I don't think we need to do that.

> +  HWND hwnd = NULL;
> +  struct frame *sf = SELECTED_FRAME ();
> +  if (FRAME_W32_P (sf) && FRAME_LIVE_P (sf))
> +    hwnd = FRAME_W32_WINDOW (sf);

If we always use the selected frame, it should be documented.  If the
same frame should be selected when removing the badge, that should
also be documented (it means the caller will need to track which frame
sets the badge).  And what happens if the caller attempts to remove
the badge when a different frame is select, or if the original frame
was deleted?

> +      if (bg_rgb == 0xff000000)
> +	{
> +	  signal_error ("Invalid background RGB", bg);
> +	  return Qnil;
> +	}
> +      if (fg_rgb == 0xff000000)
> +	{
> +	  signal_error ("Invalid foreground RGB", fg);
> +	  return Qnil;
> +	}

I'd prefer not to signal errors from these functions, but instead to
silently return nil.

> +      /* Derive a font scaled to fit the icon.  First find the system's
> +	 base font.  Then scale it to fit icon_height.  */
> +      HFONT base_font;
> +      BOOL clean_up_base_font = FALSE;
> +      if (system_parameters_info_w_fn)
> +	{
> +	  NONCLIENTMETRICSW ncm;
> +	  memset (&ncm, 0, sizeof (ncm));
> +	  ncm.cbSize = sizeof (ncm);
> +	  SystemParametersInfoW (SPI_GETNONCLIENTMETRICS, sizeof (ncm), &ncm, 0);
> +	  base_font = CreateFontIndirectW (&ncm.lfSmCaptionFont);
> +	  clean_up_base_font = TRUE;

Do we really need to call the "wide" variants of these functions?
See w32-set-wallpaper for what we do about SystemParametersInfoW, and
CreateFontIndirectW is not used anywhere in Emacs.  Can we use
CreateFontIndirect instead, or only use CreateFontIndirectW when it is
supported, like w32-set-wallpaper does with SystemParametersInfoW?

Are badges at all supported on old Windows versions?  If not, then we
can use the "wide" APIs, but we need to do that through function
pointers, which are filled at startup time using get_proc_addr, like
we do with MultiByteToWideChar.  Because otherwise older versions of
Windows will refuse to run Emacs that refers to these wide APIs which
don't exist on those old versions.

> +      /* Negative lfHeight indicates pixel units vs. positive in points.
> +	 Use the LOGPIXELSY px/in of the primary monitor.  */
> +      lf.lfHeight = -MulDiv(icon_height / 2, /* Fit ~3 chars.  */
                             ^^
> +			    72,
> +			    GetDeviceCaps(GetDC (NULL), LOGPIXELSY));
                                        ^^
Our style is to leave one space between the function's name and the
opening parenthesis.

> +      for (int y = 0; y < icon_height; ++y)
> +        for (int x = 0; x < icon_width; ++x)
> +	  {
> +            int dx = x - circle_center_x;
> +            int dy = y - circle_center_y;
> +            if (dx * dx + dy * dy <= circle_radius_sq)
> +	      {
> +		  pixel = bitmap_pixels + (y * icon_width + x);
> +		  *pixel |= 0xff000000;
                            ^^^^^^^^^^
Please add a comment explaining that "magic constant".

> +static FlashWindowEx_Proc _flash_window_ex_fn = NULL;

This variable should also be NULL-ified in globals_of_w32fns, so that
we don't risk carrying stale values from when Emacs was dumped.

> +static HWND _w32_request_user_attention_hwnd = NULL;

Again, please don't use variables whose names begin with an
underscore.

> +DEFUN ("w32-request-user-attention",
> +       Fw32_request_user_attention,
> +       Sw32_request_user_attention,
> +       1, 1, 0,
> +       doc: /* Flash the taskbar icon based on URGENCY.

AFAIU, FlashWindowEx flashes the frame itself as well, not only the
task-bar icon, doesn't it?

> +  /* dwFlags
> +     FLASHW_ALL       0x00000003 Flash both window caption and taskbar button.
> +     FLASHW_CAPTION   0x00000001 Flash the window caption.
> +     FLASHW_STOP      0x00000000 Stop flashing.
> +     FLASHW_TIMER     0x00000004 Flash continuously, until FLASHW_STOP.
> +     FLASHW_TIMERNOFG 0x0000000C Flash until window comes to the foreground.
> +     FLASHW_TRAY      0x00000002 Flash the taskbar button.  */

Should we allow flashing only the task-bar icon, as an option?

> +  flash_info.dwFlags = 0x00000000; /* FLASHW_STOP  */
> +  if (_w32_request_user_attention_hwnd != NULL
> +      && _w32_request_user_attention_hwnd != hwnd)
> +    {
> +      /* Disable attention on the old window handle.  */
> +      flash_info.hwnd = _w32_request_user_attention_hwnd;
> +      _flash_window_ex_fn (&flash_info);
> +      _w32_request_user_attention_hwnd = hwnd;

Does the fact that we store the handle to the frame's window in a
global static variable mean that we cannot flash more than one frame
at a time?

And what happens if the frame is deleted before flashing ends?

> +  if (!NILP (urgency) && SYMBOLP (urgency))
> +    {
> +      /* The intended caller, dock-mode.el 'dock-attention', has its own
> +	 timer to clear the attention indicator so this will flash until
> +	 cleared via the timer for informational attention, or the
> +	 window comes to the foreground for critical attention.  */
> +      if (EQ (urgency, Qinformational))
> +	flash_info.dwFlags = 0x00000003 | 0x00000004; /* FLASHW_TIMER  */
> +      else if (EQ (urgency, Qcritical))
> +	flash_info.dwFlags = 0x00000003 | 0x0000000C; /* FLASHW_TIMERNOFG  */
> +    }

Why are you using numerical values instead of FLASHW_ALL etc.?

> +  if (_w32_progress_indicator_hwnd != NULL
> +      && _w32_progress_indicator_hwnd != hwnd)
> +    {
> +      /* Disable the indicator for the old window handle.  */
> +      task_bar_list->lpVtbl->SetProgressState (task_bar_list,
> +					       _w32_progress_indicator_hwnd,
> +					       TBPF_NOPROGRESS);
> +    }
> +  _w32_progress_indicator_hwnd = hwnd;

Same questions as above about displaying progress for more than one
frame at a time.

> +  /* Scale task bar progress from 0.0-1.0 to 0-100.  */
> +  unsigned int adj_progress = 0;

The MS documentation says the 2 last arguments of SetProgressValue
method are of type ULONGLONG, so why not use that here?

> +  #ifdef WINDOWSNT
> +  /* Taskbar/dock */
                    ^^
Our conventions are to and a C comment with a period and two spaces
before "*/".

Previous Next

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