Subject: Re: bug#76350: [PATCH] Add MPD stats viewer to 'mpc'

From: Stefan Monnier <[email-masked]>
Date: Sun, 16 Feb 2025 23:58:14 -0500

> I wasn’t sure about what to do with the faces; reuse the existing
> names or add more so went with rename but don’t mind any of the
> options.

Rename seems fine to me.

> +*** New command 'mpc-mpd-stats'.

I think I'd rather call it `mpc-server-stats`.

>      ["Add new browser" mpc-tagbrowser]
> +    ["MPD Stats" mpc-mpd-stats]

Same here, "Server stats".

> +;;; Stats viewer ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> +
> +(defcustom mpc-mpd-stats-date-format "%c"
> +  "Format used for dates in `mpc-mpd-stats'.
> +See `format-time-string' for formatting details."
> +  :version "31.1"
> +  :type 'string)
> +
> +(defcustom mpc-mpd-stats-duration-format "%Y, %D, %z%h:%.2m:%.2s"
> +  "Format used for durations in `mpc-mpd-stats'.
> +See `format-seconds' for formatting details."
> +  :version "31.1"
> +  :type 'string)

Do we need this level of customization?
I think I'd be happy to hardcode those settings (maybe with
`defvar/defconst` with a "--" in the name).

> +         :objects (let-alist (mpc-proc-cmd-to-alist (list "stats"))
> +                    `(("Artists" ,.artists)
> +                      ("Albums" ,.albums)
> +                      ("Songs" ,.songs)
> +                      ("Play Time" ,(mpc--mpd-stats-duration .playtime))
> +                      ("Uptime" ,(mpc--mpd-stats-duration .uptime))
> +                      ("DB Play Time" ,(mpc--mpd-stats-duration .db_playtime))
> +                      ("DB Updated" ,(mpc--mpd-stats-date .db_update))))))

But here I'd be tempted not to hardcode the list, so it will always
print all the info returned by the server:

- Maybe have a generic heuristic to make the keys pretty (e.g. turn
  "albums" into "Albums" and "db_update" into "DB Update")
- Provide ad-hoc elements for those specific entries where we want to
  prettify the raw value before displaying it (i.e. where we want to
  apply `mpc--mpd-stats-duration` or `mpc--mpd-stats-date`).

WDYT?

        Stefan