search

clementine-player / Clementine

:tangerine: Clementine Music Player
https://www.clementine-player.org/
GNU General Public License v3.0
3.75k stars 676 forks source link

can't undo after "Clear playlist", clear button shouldn't be next to playback controls #6785

Open mikelpr opened 4 years ago

mikelpr commented 4 years ago

System information

Please provide information about your system and the version of Clementine used.

Expected behaviour / actual behaviour

should be able to undo after a playlist clear accidentally cleared and couldn't undo, had to recover from a db backup (thanks for that)

Steps to reproduce the problem (only for bugs)

click the clear button that totally shouldn't be next to the playback controls

gwern commented 4 months ago

This bug has bitten me many times, and it is particularly pernicious because apparently when using a tiling WM, frequently, the first focused button is the 'clear playlist' button, so at random, without any apparent pattern, switching workspaces/focus and hitting Space to play will instead erase the current playlist, without any prompt, notification, or undo, or backups. The lack of any information made it impossible to figure out what, if anything, I was doing wrong or how I was triggering the bug, and made it unreproducible: as far as I could tell, I was simply opening Clementine as usual and playing the current track, and every once in a great while, a playlist vanished into the ether.

I've only figured this out just now by accident, that the problem is misclicking a clear playlist button (?!) just now after something like 6 years of this happening intermittently (which I've been working around by exporting playlists periodically and sighing when a playlist randomly is deleted somehow).

Due to a separate graphics bug on my new system, which makes Clementine zoomed in 2x, I happened to notice a weird button I've never used was focused, indicated by ever so slight highlighting - right before my muscle memory hit Space and promptly erased everything. At which point I finally noticed there was a weird button, it was 'clear playlist' and googled to see if there was an open bug for why an ordinary built-in function apparently had no undo or even confirmation before a highly data-destructive operation. (And then it turns out this has been an open bug for 4 years now?)

So there are at least 3 fixes that ought to be made here, for a bug that should never have happened:

  1. 'Clear playlist' should not be irreversible, and should allow undo through C-z etc

  2. 'Clear playlist' should, at a minimum, prompt the user for confirmation. A status bar message would also be a good idea.

    One reason I never suspected a clear-playlist button, and kept thinking it was something about the search bar erasing or hiding all entries, is because Clementine does prompt for confirmation on other less-destructive playlist operations, like closing a playlist. So Clementine is inconsistent here.

  3. 'Clear playlist' should not be a button at all: it is not a common operation, and if it is desired, users can simply select all entries with the standard CUA C-a operation, and then Delete or right-click. If it has to be an explicit command, then it should be relegated to the menus like all the other esoteric commands.

    (It is definitely not so common an operation that it needs to be placed right next to the standard play/pause/seek buttons, which are common operations and probably ought to be larger too.)

  4. worth considering is more backups of playlists, like regular exports of the XCF.

    One of the chief commandments of programming is: "Thou Shalt Not Lose The User's Data". Disk space is now extremely cheap, the playlists are extremely small, people put a lot of time into curating them, so it would not be unreasonable at all to keep the last n revisions of each playlist, perhaps in the cache, to allow easiest restoration. (The existing config files are not backups. Binary databases are corruptible, fragile, hard to inspect, not intrinsically versioned, opaque to backup tools, and not user-friendly.)

FeepingCreature tells me that this bug does not exist in Amarok, nor does it exist in the Clementine fork strawberry (which has undo, and doesn't put it in the maximally dangerous spot), so this bug clearly was avoidable or fixable; in particular, it was apparently fixed in strawberry before this bug report was ever filed...

So it would also be good to consider a deeper root-cause analysis here: why are major actions like clear-playlist not already handled by undo? Why didn't it get a prompt to begin with? Whose idea was it to stick that right next to the play buttons? Why was the fix not imported from the strawberry fork, or at least the issue verified & added as an open bug before mikelpr ran into it the hard way?

(EDIT: given the lack of dev response, and other long-standing issues like Clementine corrupting OGG files when editing tags, I am taking the hint about clementine being abandoned, and switching to strawberry. Pretty easy switch so far.)

mikelpr commented 4 months ago

@gwern yeah I've moved on to strawberry too...