Skip to content
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

added: ability to open the smartplaylisteditor for a given playlist #6966

Closed

Conversation

notspiff
Copy link
Contributor

@Montellese
Copy link
Member

Maybe I'm missing something but why are you guessing for the type? If the XSP file exists it should also contain the type of the smartplaylist.

@akva2
Copy link
Contributor

akva2 commented Apr 16, 2015

i'm guessing based on the folder names.

special://videoplaylists/blah.xsp
special://musicplaylists/blah.xsp

sure, we could open the xsp instead in principle. in any case the editor requires that we specify this up front for which controls are visible etc. not my invention.

reason i'm guessing is that we may be creating a new file. in that case, i cannot read it from a nonexistent xsp.

@Montellese
Copy link
Member

Yes but for that you need to set m_mode which I don't see happening. And you load the playlist and then set the type even though you can already get it from the loaded playlist at that point.

@notspiff
Copy link
Contributor Author

I dont do any of those things.

@notspiff
Copy link
Contributor Author

As in read again and consider nonexistent files.

// guess type
if (type.empty())
{
if (startupList.find("video") > -1)

This comment was marked as spam.

This comment was marked as spam.

@Montellese
Copy link
Member

OK for non-existing files that makes sense. But it still looks to me like you need to

  • set m_mode (which in case of an existing smartplaylist and no type parameter could be determined from the type of the loaded smartplaylist)
  • only change the playlist type if m_playlist.Load() fails

@notspiff notspiff force-pushed the smartplaylisteditor_activatewindow branch from 5efb5e6 to 5965b43 Compare April 20, 2015 13:46
@notspiff
Copy link
Contributor Author

rebased and fixed. can't get it less ugly.

@notspiff
Copy link
Contributor Author

note that i removed the ability to open new files due to API issues (only a single string parameter is passed on from builtins/event server).

if (URIUtils::PathEquals(startupList, CProfilesManager::Get().GetUserDataItem("PartyMode-Video.xsp")))
party = 2;

if (party || m_playlist.Load(startupList))

This comment was marked as spam.

This comment was marked as spam.

@notspiff notspiff force-pushed the smartplaylisteditor_activatewindow branch from 5965b43 to c474bdd Compare April 21, 2015 07:57
@notspiff notspiff force-pushed the smartplaylisteditor_activatewindow branch from c474bdd to d8e1a32 Compare April 21, 2015 07:58
@Montellese
Copy link
Member

Should be good now, thanks.

@Montellese Montellese added the Type: Improvement non-breaking change which improves existing functionality label Apr 21, 2015
@Lunatixz
Copy link
Contributor

Thanks for adding this feature.

@Lunatixz
Copy link
Contributor

Lunatixz commented Jun 3, 2015

So will this make it into master in some form?

@NedScott
Copy link
Contributor

NedScott commented Jun 4, 2015

Most likely, yes. Spiff doesn't want to handle the PRs himself, so he went and closed them, but we're still keeping track of them. We just have to wait for master to be un-feature frozen.

@Lunatixz
Copy link
Contributor

Lunatixz commented Jun 4, 2015

Thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement non-breaking change which improves existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants