GNU bug report logs - #74200
[PATCH] Add song viewer to 'mpc'

Previous Next

Full log

View this message in rfc822 format

From: john muhl <jm <at> pub.pink>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 74200 <at> debbugs.gnu.org
Subject: bug#74200: [PATCH] Add song viewer to 'mpc'
Date: Tue, 12 Nov 2024 19:01:17 -0600

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

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

>> +  "i"                           #'mpc-describe-song)
>
> What about using "d" (i.e. the "describe" mnemonic instead of the "info"
> mnemonic)?

Done.

>> +(defconst mpc-song-viewer-tagtypes
>> +  (sort (append '("Bitrate" "Duration" "File" "Format") (mpc-cmd-tagtypes)))
>> +  "Tag types available for use in `mpc-song-viewer-tags'.")
>
> Hmm... running `mpc-cmd-tagtypes` when we load the file seems risky: we
> may not know the `mpc-host` yet, or any other thing may go wrong.

Right you are. The tags are always the same anyway so I put a note
in the docstring and got rid of the risky constant.

>> +(defun mpc-describe-song (&optional file)
>
> Any reason we can't make the argument mandatory?

Nope.

>> +    (when tags
>> +      (with-current-buffer (get-buffer-create buffer)
>> +        (special-mode)
>> +        (visual-line-mode)
>> +        (let ((buffer-read-only nil))
>
> This should bind `inhibit-read-only` instead.

Fixed.

>> +      (select-window (display-buffer buffer '((display-buffer-reuse-window
>> +                                               display-buffer-same-window)
>> +                                              (reusable-frames . t)))))))
>
> Why use `select-window + display-buffer` instead of `pop-to-buffer`?

Elsewhere I was told not to use pop-to-buffer in new code and I
believe everything anyone tells me.

[0001-Add-song-viewer-to-mpc-Bug-74200.patch (text/x-patch, attachment)]

Previous Next

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