Hi Petteri,
Petteri Hintsanen <[email protected]> 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 mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/emms-help
