Double-clicked playlists in playlist pane now replace current playlist if enabled in options

[dwbapst]

Addresses #683

Playlists double-clicked in the Playlists pane, when 'double-click in lefthand pane replaces current' option is enabled, now replace the current playlist entirely. This also applies when a particular track under a particular playlist is double-clicked - that track will replace the existing playlist as well, just as if I had double-clicked that track in Collection.

PS: I am an R developer of ten years, and this is the first time I have ever edited Python code (even though I helped teach several Python workshops). I apologize for any striking errors, frankly I am amazed I was even able to do this.

[dwbapst]

My pull request with the most recent build now passes all checks - is there anything I need to do, or should do to proceed further, such as formally requesting review?

[rokm]

Sorry for the delay! Functionality-wise the proposed changes look good - the contents of current tab are now properly replaced when either a smart playlist, a playlist, or a track from a playlist are double clicked and "Replace content on side pane double click" is enabled.

I have two comments, though:

1. Seeing as the PR consists of one main commit and several follow-up fixes, can you please squash them all into a single commit? (You can do an interactive rebase on your local tree, and then force push to branch to update the PR).

2. The added playlist-replacement codepath introduces an additional way for retrieving the selected playlist via self.tree.get_selected_page(). Is there a particular reason for doing so, and not simply re-using the item from the existing codepath?
   In other words, leaving the smart playlist vs. playlist selection as it is, and then either emitting playlist-selected, or gathering tracks from item and emitting replace-items?

[dwbapst]

Thanks, @rokm ! I'm glad to hear it looks good from a functional perspective. I'll squash the commits as soon as I can.

I'm not sure I understand your second point. Could you elaborate a bit more? Like I said, this is the first time I've coded in Python rather than R, so I think its just a vocabulary issue.

[rokm]

I'm not sure I understand your second point. Could you elaborate a bit more? Like I said, this is the first time I've coded in Python rather than R, so I think its just a vocabulary issue.

When replacement is enabled, the selected playlist lookup is duplicated:

selected_playlist = self.tree.get_selected_page()

At that point, the selected playlist should already be available in the item variable.
The item might contain a smart playlist, and would thus require an additional
get_playlist() call - which is what the existing code (in the non-replace branch)
is doing. (The get_selected_page() also does that behind the scenes, but it also quietly ignores errors.)

So I was wondering if it would not be better to simply reuse the existing playlist lookup and only adjust the action at the end:

def open_item(self, tree, path, col):
    """
        Called when the user double clicks on a playlist,
        also called when the user double clicks on a track beneath
        a playlist.  When they active a track it opens the playlist
        and starts playing that track
    """
    iter = self.model.get_iter(path)
    item = self.model.get_value(iter, 2)

    replace = settings.get_option('playlist/replace_content', False)

    if item is not None:
        if isinstance(item, (Playlist, SmartPlaylist)):            
            # for smart playlists
            if hasattr(item, 'get_playlist'):
                try:
                    item = item.get_playlist(self.collection)
                except Exception as e:
                    logger.exception("Error loading smart playlist")
                    dialogs.error(
                        self.parent,
                        _("Error loading smart playlist: %s") % str(e),
                    )
                    return
            else:
                # Get an up to date copy
                item = self.playlist_manager.get_playlist(item.name)
                # item.set_is_custom(True)

            if replace:
                tracks = [track for track in item]
                self.emit('replace-items', tracks)
            else:
                self.emit('playlist-selected', item)
        else:
            if replace:
                self.emit('replace-items', [item.track])
            else:
                self.emit('append-items', [item.track], True)