isamert / empv.el

An Emacs media player, media library manager, radio player, YouTube frontend
GNU General Public License v3.0
102 stars 18 forks source link

possibly redundant :type in defcustom #51

Closed tvraman closed 7 months ago

tvraman commented 7 months ago

s ee https://github.com/isamert/empv.el/blob/main/empv.el#start-of-content :type may be repeated

isamert commented 7 months ago

Hey @tvraman, The link is broken, can you send the defcustom name or fix the link? Thanks.

tvraman commented 7 months ago

apologies for the broken link, I meant this one:

(defcustom empv-display-current-format "[#{state}#{item-loop-indicator}, #{time-pos} of #{duration} (#{percent-pos}%), #{playlist-pos}/#{playlist-count}#{playlist-loop-indicator}#{radio}#{volume}#{speed}] #{title} #{chapter}" "Format of the message displayed when `empv-display-current' is called.

This is a string representing the format or this can be an alist from file format to format string, like:

\\='((\"hls\" . \"#{state} #{title} ...\")
  (\"mp3\" . \"#{state} #{title} ...\")
  ;; Multiple formats can be defined like this:
  (\"mkv,mp4\" . \"#{state} #{title} ...\")
  ;; Catch-all case:
  (t . \"#{state} #{title} ...\"))

So that you can use different formats for different file types.

Here are the template strings that you can utilize in the format string:

isamert commented 7 months ago

No problem! Which part of the :type can become repeated? It can be either a string or alist, which the current :type describes. Maybe can you write the full :type definition you have in mind so that I can compare. I am not well accustomed with :type on defcustoms, so I might be missing something.

tvraman commented 7 months ago

I think you can just drop the :type 'string.

Here is what I was thinking; agreed defcustom is a somewhat hairy mess. I'll comment out the line I think you can delete with the prefix ;;;tvr:.

Reason: The type is a choice, one of the choices is already 'string.

(defcustom empv-display-current-format "[#{state}#{item-loop-indicator}, #{time-pos} of #{duration} (#{percent-pos}%), #{playlist-pos}/#{playlist-count}#{playlist-loop-indicator}#{radio}#{volume}#{speed}] #{title} #{chapter}" "Format of the message displayed when `empv-display-current' is called.

This is a string representing the format or this can be an alist from file format to format string, like:

\\='((\"hls\" . \"#{state} #{title} ...\")
  (\"mp3\" . \"#{state} #{title} ...\")
  ;; Multiple formats can be defined like this:
  (\"mkv,mp4\" . \"#{state} #{title} ...\")
  ;; Catch-all case:
  (t . \"#{state} #{title} ...\"))

So that you can use different formats for different file types.

Here are the template strings that you can utilize in the format string:

isamert commented 7 months ago

Oh, got it (took a while to see that line, I was skipping it unconsciously). That shouldn't be there. Thanks!