Subject: Re: bug#73538: [PATCH] Add notifications support to 'mpc'
From: john muhl <[email-masked]>
To: Stefan Monnier <[email-masked]>
Cc: [email-masked]
Date: Thu, 17 Oct 2024 19:26:03 -0500
List-Id: <debbugs-submit.debbugs.gnu.org>

Stefan Monnier <[email-masked]> writes:

>> Here’s an updated patch. Should have all feedback incorporated and
>> now with a NEWS entry.
>
> Thanks, merged into `master` (after which I installed some further tweaks).

Thanks for your time. I’ll have another one for you to look at
soonish.

>>> Question: Why is it interactive?
>> I have a global mpc-minor-mode that adds a key binding for
>> notifications and it must have bled through.
>
> 🙂
>
>> Instead of changing mpc-format I made the customizable variables
>> take a list of specs where the first element to return something
>> interesting is used and a plain string can be added for fallback:
>>
>>   (setopt mcp-notifications-body-specs
>>           '("%{Artist}" "%{AlbumArtist}" "Unknown Artist"))
>>
>> I added your description of the FORMAT-SPEC to the mpc-format
>> docstring too.
>
> I tweaked the docstring a bit further.  Regarding the
> `mpc-notifications-body/title`, the problem I can see with it is that
> the "conditionality" is based on the fact that the overall result is
> an empty string, so you can use something like
>
>     (setopt mcp-notifications-body
>             '("By %{Artist}" "By %{AlbumArtist}" "Unknown Artist"))
>
> since the first will expand to the non-empty "By ".  🙁

Whoops. I was trying to mimic how other players do it and
overlooked that possibility since I don’t think I’ve ever seen
something like that. I guess there is still the chance to tweak it
in time for 31 in case the complaints come pouring in.