[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [emms-help] [PATCH] Improve browsing by artist and year
From: |
Rasmus |
Subject: |
Re: [emms-help] [PATCH] Improve browsing by artist and year |
Date: |
Tue, 20 Oct 2015 10:18:24 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) |
Hi Petteri,
Petteri Hintsanen <address@hidden> writes:
>>> +(defcustom emms-browser-prefer-albumartist nil
>>> + "*Prefer albumartist tag.
>
>>> +If non-nil, emms-browser-get-track-field-extended prefers
>>> +albumartist tag over artist tag, if available."
>>> + :group 'emms-browser
>>> + :type 'boolean)
>>
>> Please make this t. I'd rather call the next EMMS v5 than hold back
>> improvements.
> ...
>>> +(defcustom emms-browser-prefer-originalyear nil
>>
>> It’s not obvious that this should be off by default. Presumably, /if/ you
>> have this tag, it should be used as long as the normal year tag is used as
>> fallback .
>
> OK, maybe both should be t by default. I'm usually very conservative
> about the defaults. Backwards compatibility is valuable.
It is. But here you are adding a new feature that need not interfere. If
my audio does not have originalyear it’ll fall back on year anyway. Same
for albumartist.
For albumartist I feel strongly that it should be on by default, as albums
may be broken up between different artists otherwise. For originalyear my
gut-feeling is that it will be nil unless explicitly and willingly set.
In that case, there’s probably a desire to use it. I feel less strongly
about this.
>>> +(defun emms-browser-get-track-field-extended (track type)
>> IMO: This should be merged with emms-browser-get-track-field. As long as
>> the preferred type of year and artist can be set there’s no reason to have
>> two functions.
>
> emms-get-track-field is a wrapper that just funcalls
> emms-browser-get-track-field-function. According to the docstring,
> emms-browser-get-track-field-function is meant for "customizing the
> way data is organized in the browser." As such I think that it is
> appropriate to provide extended functionality via separate function,
> hence emms-browser-get-track-field-extended.
Thanks for the reminder. So I guess what I want is to change
emms-browser-get-track-field-function value.
What I would like to avoid is the need to set the {albumartist,
orginialyear} defcustoms in vain due to the wrong extractor function is
used. This is hard to archive without going against the spirit of the
emms-browser code.
I favor something that behaves like emms-browser-get-track-field-simple
when all defcustoms are off, but I fear it would add unnecessary
complexity.
> It needs to be documented though.
Yes, and you also need to update the emms/NEWS file. One bullet per
change relevant to users.
> First I was thinking about modifying emms-track-get directly to
> replace year with originalyeal and artist with albumartist there (if
> prefer variables are non-nil). But I think that it would be too
> invasive change. (Compatibility worry again.)
Indeed. Changes should stay within emms-browser IMO.
Rasmus
--
When the facts change, I change my mind. What do you do, sir?
[emms-help] [PATCH] Add browsing by record label, Petteri Hintsanen, 2015/10/18
Re: [emms-help] [PATCH] New metadata extraction program, Rasmus, 2015/10/19