GNU bug report logs - #76350
[PATCH] Add MPD stats viewer to 'mpc'

Previous Next

Package: emacs;

Reported by: jm <at> pub.pink

Date: Sun, 16 Feb 2025 20:07:01 UTC

Severity: normal

Tags: patch

Done: Stefan Monnier <monnier <at> iro.umontreal.ca>

To reply to this bug, email your comments to 76350 AT debbugs.gnu.org.
There is no need to reopen the bug first.

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to monnier <at> iro.umontreal.ca, bug-gnu-emacs <at> gnu.org:
bug#76350; Package emacs. (Sun, 16 Feb 2025 20:07:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to jm <at> pub.pink:
New bug report received and forwarded. Copy sent to monnier <at> iro.umontreal.ca, bug-gnu-emacs <at> gnu.org. (Sun, 16 Feb 2025 20:07:02 GMT) Full text and rfc822 format available.

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

From: jm <at> pub.pink
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add MPD stats viewer to 'mpc'
Date: Sun, 16 Feb 2025 14:05:55 -0600
Tags: patch

This adds a viewer for mpd stats.

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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76350; Package emacs. (Sun, 16 Feb 2025 20:10:02 GMT) Full text and rfc822 format available.

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

From: jm <at> pub.pink
To: 76350 <at> debbugs.gnu.org
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#76350: [PATCH] Add MPD stats viewer to 'mpc'
Date: Sun, 16 Feb 2025 14:08:38 -0600
[0001-Add-MPD-stats-viewer-to-mpc-Bug-76350.patch (text/x-patch, attachment)]
[Message part 2 (text/plain, inline)]

jm <at> pub.pink writes:

> Tags: patch
>
> This adds a viewer for mpd stats.
>
> 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.


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76350; Package emacs. (Mon, 17 Feb 2025 04:59:02 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: jm <at> pub.pink
Cc: 76350 <at> debbugs.gnu.org
Subject: Re: bug#76350: [PATCH] Add MPD stats viewer to 'mpc'
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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76350; Package emacs. (Tue, 18 Feb 2025 01:05:01 GMT) Full text and rfc822 format available.

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

From: john muhl <jm <at> pub.pink>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76350 <at> debbugs.gnu.org
Subject: Re: bug#76350: [PATCH] Add MPD stats viewer to 'mpc'
Date: Mon, 17 Feb 2025 15:03:51 -0600
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> +;;; 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 hope not.

>> +         :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?

Sounds good. How about the attached?

[0001-Add-MPD-stats-viewer-to-mpc-Bug-76350.patch (text/x-patch, attachment)]

Reply sent to Stefan Monnier <monnier <at> iro.umontreal.ca>:
You have taken responsibility. (Tue, 18 Feb 2025 03:42:01 GMT) Full text and rfc822 format available.

Notification sent to jm <at> pub.pink:
bug acknowledged by developer. (Tue, 18 Feb 2025 03:42:02 GMT) Full text and rfc822 format available.

Message #19 received at 76350-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: john muhl <jm <at> pub.pink>
Cc: 76350-done <at> debbugs.gnu.org
Subject: Re: bug#76350: [PATCH] Add MPD stats viewer to 'mpc'
Date: Mon, 17 Feb 2025 22:41:02 -0500
> Sounds good. How about the attached?

Pushed to `master`, thanks!


        Stefan





This bug report was last modified 3 days ago.

Previous Next


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