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

Clarify purpose of browsers/playlists/main.py: __check_current #2446

Closed
Meriipu opened this issue Jun 21, 2017 · 3 comments
Closed

Clarify purpose of browsers/playlists/main.py: __check_current #2446

Meriipu opened this issue Jun 21, 2017 · 3 comments

Comments

@Meriipu
Copy link
Contributor

Meriipu commented Jun 21, 2017

In browsers/playlists/main.py, what does the method __check_current do (or rather, what is its purpose) ?
https://github.com/quodlibet/quodlibet/blob/master/quodlibet/quodlibet/browsers/playlists/main.py#L304

    def __check_current(self, model, path, iter):
        model, citer = self.__selected_playlists()
        if citer and model.get_path(citer) == path:
            songlist = qltk.get_top_parent(self).songlist
            self.activate(resort=not songlist.is_sorted())

Not running it seems to fix #2253 for me, but I noticed at least one thing that breaks:
I am in the playlist browser (ctrl+2), and have a playlist selected. If I drag and drop a track to the playlist I am in from the tracklist in the playlist (so to add a duplicate track), the playlist does not update with the new track until I select a different playlist.

Other than that I have not noticed anything that breaks.

__check_current (only?) seems to be called as a result of the signal row-changed.

@Meriipu Meriipu changed the title (Question) purpose of playlists/main.py: __check_current (Question) purpose of browsers/playlists/main.py: __check_current Jun 21, 2017
@lazka
Copy link
Member

lazka commented Jun 21, 2017

For such cases it's always good to have a look at the blame output: https://github.com/quodlibet/quodlibet/blame/master/quodlibet/quodlibet/browsers/playlists/main.py

row-changed is not the best place for this as it's usually just meant for updating the view. +1 for getting rid of it.

@Meriipu
Copy link
Contributor Author

Meriipu commented Jun 22, 2017

I have no idea how it could be changed though.
If the only thing that breaks is that the playlist is not reloaded when it grows (or shrinks?) then maybe an extra check for whether the playlist actually grew or shrunk, or a separate callback for that and removing this.

I am not very into gtk concepts like signals, eventsl and updates at all though.

@declension declension changed the title (Question) purpose of browsers/playlists/main.py: __check_current Clarify purpose of browsers/playlists/main.py: __check_current Jun 22, 2017
@Meriipu
Copy link
Contributor Author

Meriipu commented Jul 17, 2017

since it was removed with that commit I suppose this can be closed unless maybe the reason it was there in the first place resurfaces.

@Meriipu Meriipu closed this as completed Jul 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants