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

show confirmation dialog when removing songs #2667

Merged
merged 3 commits into from Dec 27, 2017

Conversation

Projects
None yet
2 participants
@zsau
Contributor

zsau commented Dec 26, 2017

Fixes #2248. Not sure I did the testing right.

title = _("Remove track: \"%s\" from library?"
% next(iter(songs))['title'])
else:
title = _("Remove %s tracks from library?" % len(songs))

This comment has been minimized.

@declension

declension Dec 27, 2017

Member

This needs to use the ngettext (plural) form - I know you check for n=1, but languages behave quite differently with their pluralisation (e.g. some have several plural forms, and n=0 varies (though I guess this should never happen)).

Also that should really be a %d...

songs = set(songs)
if len(songs) == 1:
title = _("Remove track: \"%s\" from library?"
% next(iter(songs))['title'])

This comment has been minimized.

@declension

declension Dec 27, 2017

Member

Perhaps we should consider the possible-though-rare case where tracks don't have titles. We could default to the ~filename in this case, perhaps, rather than display ""

@zsau

This comment has been minimized.

Contributor

zsau commented Dec 27, 2017

I should also credit @Meriipu for this, since it's based on their gist from #2667 (https://gist.github.com/Meriipu/f59d3a0871b528d58fc9f2ad4e19bec3).

@declension declension merged commit 3903166 into quodlibet:master Dec 27, 2017

6 checks passed

ci/circleci: job.fedora26 Your tests passed on CircleCI!
Details
ci/circleci: job.ubuntu16.04 Your tests passed on CircleCI!
Details
ci/circleci: job.ubuntu17.10 Your tests passed on CircleCI!
Details
ci/circleci: job.win32 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@declension

This comment has been minimized.

Member

declension commented Dec 27, 2017

Thanks!

@zsau zsau deleted the zsau:removal_confirmation branch Dec 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment