-
Notifications
You must be signed in to change notification settings - Fork 18
Always set 'title' element #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Always set 'title' element #104
Conversation
Signed-off-by: darrell-k <[email protected]>
|
This should be handled by XMLBrowser. See my comment in the other PR. |
Signed-off-by: darrell-k <[email protected]>
|
Changes as discussed in #103 |
Plugin.pm
Outdated
| } | ||
|
|
||
| if ( $prefs->get('parentalWarning') && $track->{parental_warning} ) { | ||
| $item->{title} .= ' [E]'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's really try to leave the title alone. This should be the title and nothing else, as it might be used to look up additional metadata etc. Let's see whether anyone would miss that additional piece of information. The same applies to the asterisk for the samples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "explicit" indicator was a user-requested enhancement (there's also the * added for not-yet streamable tracks). Same argument as above applies: if it goes into line1 it needs to be in title.
But in this case, we could set a different element with these indicators, with corresponding client changes to append to the displayed title.
The [E] and * indicators may also be added to the album name. I think the solution at the album level would be to include in a version element (again with the necessary client changes). This is related to LMS-Community/slimserver#1398 which I need to finalise (although changing this for the plugin would not be dependent on that PR).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be in support of an extra element for such additions. As you might know I'm often struggling trying to match names and titles to whatever a web site might be using. And all these additions complicate things considerably.
Signed-off-by: darrell-k <[email protected]>
|
Just added a commit to keep album name and track title clean by adding elements: |
Signed-off-by: darrell-k <[email protected]>
I'm just curious as to why the "[E]" and "*" modifiers are added to the |
michaelherger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
michaelherger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Because unlike name/title, the new element starts off empty. |
Something's happening in XMLBrowser which messes up the title format when the extra metadata is passed back, so only return it when we've got the
$tagsparameter.Maybe we should look at XMLBrowser, in case we ever wanted to use
$tagsin Default Skin requests?