Re: [PATCH v3] notifications: Allow to use Icon Naming Specification for app-icon

[Michael Albinus <michael.albinus@gmx.de>]

Andrew Tropin <andrew@trop.in> writes:

Hi Andrew,

Eli did comment already (I agree with him). Just some few further nits.

> Icon is not always a file name, but can be just an icon name.
> https://specifications.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html
>
> diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
> index 4bcc9d5fea6..efa89fa387c 100644
> --- a/doc/lispref/os.texi
> +++ b/doc/lispref/os.texi
> @@ -2885,6 +2885,9 @@ Desktop Notifications
>  @item :app-icon @var{icon-file}
>  The file name of the notification icon.  If set to @code{nil}, no icon
>  is displayed.  The default is @code{notifications-application-icon}.
> +If value is a string @code{expand-file-name} will be applied, if value
> +is a symbol it will be used as is (helpful when using Icon Naming
> +Specification).

Please add the URL of the spec above, perhaps as @footnote. Furthermore,
icon names are not restricted to the ones from the Icon Naming
Specification, you can use any icon name which is in the user's current icon
theme. The specification tells you what shall be offered on any system. For
example, you could say now

  (notifications-notify :title "The thunderbird icon" :app-icon 'thunderbird)

I have installed thunderbird, of course. And I don't need to care that
the corresponding icon files are installed at /usr/share/icons/**/apps/thunderbird.png

> diff --git a/etc/NEWS b/etc/NEWS
> index 5883b4df2a7..e1a5cbf0340 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -548,6 +548,13 @@ Similarly to buffer restoration by Desktop, 'recentf-mode' checking
>  of the accessibility of remote files can now time out if
>  'remote-file-name-access-timeout' is set to a positive number.
>
> +** Notifications
> +
> ++++
> +*** Allow to use Icon Naming Specification for app-icon.
> +Symbol can be used as a value for :app-icon to provide icon name
> +without specifying a file.

Say ':app-icon'.

I found your initial example funny, maybe add it here:

  (notifications-notify
    :title "I am playing music" :app-icon 'multimedia-player)

Best regards, Michael.