Re: [emms-help] [PATCH] Improve browsing by artist and year

[Rasmus]

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?