actions: get deleting/cropping/clearing playlists right

src/actions.cpp

@@ -771,24 +771,12 @@ void VolumeDown::Run()
 
 void Delete::Run()
 {
-	using Global::wFooter;
-	using MPD::itDirectory;
-	using MPD::itSong;
-	using MPD::itPlaylist;
-	
-	if (!myPlaylist->Items->empty() && myScreen == myPlaylist)
+	if (myScreen == myPlaylist && !myPlaylist->Items->empty())
 	{
-		auto list = getSelectedOrCurrent(myPlaylist->Items->begin(), myPlaylist->Items->end(), myPlaylist->Items->currentI());
-		Mpd.StartCommandsList();
-		for (auto it = list.rbegin(); it != list.rend(); ++it)
-			Mpd.DeleteID((*it)->value().getID());
-		if (Mpd.CommitCommandsList())
-		{
-			for (auto it = list.begin(); it != list.end(); ++it)
-				(*it)->setSelected(false);
-			if (list.size() > 1)
-				ShowMessage("Selected items deleted");
-		}
+		ShowMessage("Deleting items...");
+		auto delete_fun = std::bind(&MPD::Connection::Delete, _1, _2);
+		if (deleteSelectedSongs(*myPlaylist->Items, delete_fun))
+			ShowMessage("Item(s) deleted");
 	}
 #	ifndef WIN32
 	else if (myScreen == myBrowser && !myBrowser->Main()->empty())
@@ -802,7 +790,7 @@ void Delete::Run()
 		else
 		{
 			MPD::Item &item = myBrowser->Main()->current().value();
-			std::string name = item.type == itSong ? item.song->getName() : item.name;
+			std::string name = item.type == MPD::itSong ? item.song->getName() : item.name;
 			question = "Delete ";
 			question += itemTypeToString(item.type);
 			question += " \"";
@@ -817,7 +805,7 @@ void Delete::Run()
 			for (auto it = list.begin(); it != list.end(); ++it)
 			{
 				const MPD::Item &i = (*it)->value();
-				std::string name = i.type == itSong ? i.song->getName() : i.name;
+				std::string name = i.type == MPD::itSong ? i.song->getName() : i.name;
 				if (myBrowser->deleteItem(i))
 				{
 					const char msg[] = "\"%s\" deleted";
@@ -839,7 +827,6 @@ void Delete::Run()
 		}
 		else
 			ShowMessage("Aborted");
-			
 	}
 #	endif // !WIN32
 	else if (myScreen == myPlaylistEditor && !myPlaylistEditor->Content->empty())
@@ -870,19 +857,11 @@ void Delete::Run()
 		}
 		else if (myScreen->ActiveWindow() == myPlaylistEditor->Content)
 		{
-			// select current song so we won't lost track of it after filtering is canceled
-			selectCurrentIfNoneSelected(*myPlaylistEditor->Content);
-			myPlaylistEditor->Content->showAll();
-			auto list = getSelected(myPlaylistEditor->Content->begin(), myPlaylistEditor->Content->end());
-			Mpd.StartCommandsList();
-			auto begin = myPlaylistEditor->Content->begin();
-			for (auto it = list.rbegin(); it != list.rend(); ++it)
-				Mpd.Delete(myPlaylistEditor->Playlists->current().value(), *it-begin);
-			if (Mpd.CommitCommandsList())
-			{
-				if (list.size() > 1)
-					ShowMessage("Selected items deleted");
-			}
+			std::string playlist = myPlaylistEditor->Playlists->current().value();
+			auto delete_fun = std::bind(&MPD::Connection::PlaylistDelete, _1, playlist, _2);
+			ShowMessage("Deleting items...");
+			if (deleteSelectedSongs(*myPlaylistEditor->Content, delete_fun))
+				ShowMessage("Item(s) deleted");
 		}
 	}
 }
@@ -1014,7 +993,8 @@ void MoveSelectedItemsUp::Run()
 	{
 		assert(!myPlaylistEditor->Playlists->empty());
 		std::string playlist = myPlaylistEditor->Playlists->current().value();
-		moveSelectedItemsUp(*myPlaylistEditor->Content, std::bind(&MPD::Connection::PlaylistMove, _1, playlist, _2, _3));
+		auto move_fun = std::bind(&MPD::Connection::PlaylistMove, _1, playlist, _2, _3);
+		moveSelectedItemsUp(*myPlaylistEditor->Content, move_fun);
 	}
 }
 
@@ -1038,7 +1018,8 @@ void MoveSelectedItemsDown::Run()
 	{
 		assert(!myPlaylistEditor->Playlists->empty());
 		std::string playlist = myPlaylistEditor->Playlists->current().value();
-		moveSelectedItemsDown(*myPlaylistEditor->Content, std::bind(&MPD::Connection::PlaylistMove, _1, playlist, _2, _3));
+		auto move_fun = std::bind(&MPD::Connection::PlaylistMove, _1, playlist, _2, _3);
+		moveSelectedItemsDown(*myPlaylistEditor->Content, move_fun);
 	}
 }
 
@@ -1053,7 +1034,12 @@ void MoveSelectedItemsTo::Run()
 	if (myScreen == myPlaylist)
 		moveSelectedItemsTo(*myPlaylist->Items, std::bind(&MPD::Connection::Move, _1, _2, _3));
 	else
-		moveSelectedItemsTo(*myPlaylistEditor->Content, std::bind(&MPD::Connection::PlaylistMove, _1, myPlaylistEditor->Playlists->current().value(), _2, _3));
+	{
+		assert(!myPlaylistEditor->Playlists->empty());
+		std::string playlist = myPlaylistEditor->Playlists->current().value();
+		auto move_fun = std::bind(&MPD::Connection::PlaylistMove, _1, playlist, _2, _3);
+		moveSelectedItemsTo(*myPlaylistEditor->Content, move_fun);
+	}
 }
 
 bool Add::canBeRun() const
@@ -1819,32 +1805,14 @@ void AddSelectedItems::Run()
 
 void CropMainPlaylist::Run()
 {
-	if (myPlaylist->isFiltered())
-		return;
 	bool yes = true;
 	if (Config.ask_before_clearing_main_playlist)
 		yes = AskYesNoQuestion("Do you really want to crop main playlist?", TraceMpdStatus);
 	if (yes)
 	{
-		bool delete_all_but_current = !myPlaylist->Items->hasSelected();
-		Mpd.StartCommandsList();
-		int current = myPlaylist->Items->choice();
-		for (int i = myPlaylist->Items->size()-1; i >= 0; --i)
-		{
-			bool delete_i = (delete_all_but_current && i != current)
-			             || (!delete_all_but_current && !myPlaylist->Items->at(i).isSelected());
-			if (delete_i && i != myPlaylist->NowPlaying)
-				Mpd.Delete(i);
-		}
-		// if mpd deletes now playing song deletion will be sluggishly slow
-		// then so we have to assure it will be deleted at the very end.
-		bool delete_np = (delete_all_but_current && current != myPlaylist->NowPlaying)
-		              || (!delete_all_but_current && !myPlaylist->Items->at(myPlaylist->NowPlaying).isSelected());
-		if (myPlaylist->isPlaying() && delete_np)
-			Mpd.DeleteID(myPlaylist->NowPlayingSong()->getID());
 		ShowMessage("Cropping playlist...");
-		if (Mpd.CommitCommandsList())
-			ShowMessage("Playlist cropped");
+		if (cropPlaylist(*myPlaylist->Items, std::bind(&MPD::Connection::Delete, _1, _2)))
+			ShowMessage("Cropping playlist...");
 	}
 }
 
@@ -1855,31 +1823,17 @@ bool CropPlaylist::canBeRun() const
 
 void CropPlaylist::Run()
 {
-	if (myPlaylistEditor->Playlists->empty() || myPlaylistEditor->isContentFiltered())
-		return;
+	assert(!myPlaylistEditor->Playlists->empty());
+	std::string playlist = myPlaylistEditor->Playlists->current().value();
 	bool yes = true;
 	if (Config.ask_before_clearing_main_playlist)
-		yes = AskYesNoQuestion("Do you really want to crop playlist \"" + myPlaylistEditor->Playlists->current().value() + "\"?", TraceMpdStatus);
+		yes = AskYesNoQuestion("Do you really want to crop playlist \"" + playlist + "\"?", TraceMpdStatus);
 	if (yes)
 	{
-		bool delete_all_but_current = !myPlaylistEditor->Content->hasSelected();
-		Mpd.StartCommandsList();
-		int current = myPlaylistEditor->Content->choice();
-		std::string playlist = locale_to_utf_cpy(myPlaylistEditor->Playlists->current().value());
-		for (int i = myPlaylistEditor->Content->size()-1; i >= 0; --i)
-		{
-			bool delete_i = (delete_all_but_current && i != current)
-			             || (!delete_all_but_current && !myPlaylistEditor->Content->at(i).isSelected());
-			if (delete_i)
-				Mpd.Delete(playlist, i);
-		}
-		ShowMessage("Cropping playlist \"%s\"...", myPlaylistEditor->Playlists->current().value().c_str());
-		if (Mpd.CommitCommandsList())
-		{
-			ShowMessage("Playlist \"%s\" cropped", myPlaylistEditor->Playlists->current().value().c_str());
-			// enforce content update
-			myPlaylistEditor->Content->clear();
-		}
+		auto delete_fun = std::bind(&MPD::Connection::PlaylistDelete, _1, playlist, _2);
+		ShowMessage("Cropping playlist \"%s\"...", playlist.c_str());
+		if (cropPlaylist(*myPlaylistEditor->Content, delete_fun))
+			ShowMessage("Playlist \"%s\" cropped", playlist.c_str());
 	}
 }
 
@@ -1890,21 +1844,11 @@ void ClearMainPlaylist::Run()
 		yes = AskYesNoQuestion("Do you really want to clear main playlist?", TraceMpdStatus);
 	if (yes)
 	{
-		if (myPlaylist->Items->isFiltered())
-		{
-			ShowMessage("Deleting filtered items...");
-			Mpd.StartCommandsList();
-			for (int i = myPlaylist->Items->size()-1; i >= 0; --i)
-				Mpd.Delete((*myPlaylist->Items)[i].value().getPosition());
-			if (Mpd.CommitCommandsList())
-				ShowMessage("Filtered items deleted");
-		}
-		else
-		{
-			ShowMessage("Clearing playlist...");
-			if (Mpd.ClearPlaylist())
-				ShowMessage("Playlist cleared");
-		}
+		auto delete_fun = std::bind(&MPD::Connection::Delete, _1, _2);
+		auto clear_fun = std::bind(&MPD::Connection::ClearMainPlaylist, _1);
+		ShowMessage("Deleting items...");
+		if (clearPlaylist(*myPlaylist->Items, delete_fun, clear_fun))
+			ShowMessage("Items deleted");
 	}
 }
 
@@ -1915,16 +1859,18 @@ bool ClearPlaylist::canBeRun() const
 
 void ClearPlaylist::Run()
 {
-	if (myPlaylistEditor->Playlists->empty() || myPlaylistEditor->isContentFiltered())
-		return;
+	assert(!myPlaylistEditor->Playlists->empty());
+	std::string playlist = myPlaylistEditor->Playlists->current().value();
 	bool yes = true;
 	if (Config.ask_before_clearing_main_playlist)
-		yes = AskYesNoQuestion("Do you really want to clear playlist \"" + myPlaylistEditor->Playlists->current().value() + "\"?", TraceMpdStatus);
+		yes = AskYesNoQuestion("Do you really want to clear playlist \"" + playlist + "\"?", TraceMpdStatus);
 	if (yes)
 	{
-		ShowMessage("Clearing playlist \"%s\"...", myPlaylistEditor->Playlists->current().value().c_str());
-		if (Mpd.ClearPlaylist(locale_to_utf_cpy(myPlaylistEditor->Playlists->current().value())))
-			ShowMessage("Playlist \"%s\" cleared", myPlaylistEditor->Playlists->current().value().c_str());
+		auto delete_fun = std::bind(&MPD::Connection::PlaylistDelete, _1, playlist, _2);
+		auto clear_fun = std::bind(&MPD::Connection::ClearPlaylist, _1, playlist);
+		ShowMessage("Deleting items from \"%s\"...", playlist.c_str());
+		if (clearPlaylist(*myPlaylistEditor->Content, delete_fun, clear_fun))
+			ShowMessage("Items deleted from \"%s\"", playlist.c_str());
 	}
 }
 

src/helpers.h

@@ -50,7 +50,8 @@ inline MPD::Song *currentSong(BasicScreen *screen)
 	return ptr;
 }
 
-template <typename Iterator> bool hasSelected(Iterator first, Iterator last)
+template <typename Iterator>
+bool hasSelected(Iterator first, Iterator last)
 {
 	for (; first != last; ++first)
 		if (first->isSelected())
@@ -58,7 +59,8 @@ template <typename Iterator> bool hasSelected(Iterator first, Iterator last)
 	return false;
 }
 
-template <typename Iterator> std::vector<Iterator> getSelected(Iterator first, Iterator last)
+template <typename Iterator>
+std::vector<Iterator> getSelected(Iterator first, Iterator last)
 {
 	std::vector<Iterator> result;
 	for (; first != last; ++first)
@@ -67,7 +69,8 @@ template <typename Iterator> std::vector<Iterator> getSelected(Iterator first, I
 	return result;
 }
 
-template <typename T> void selectCurrentIfNoneSelected(NC::Menu<T> &m)
+template <typename T>
+void selectCurrentIfNoneSelected(NC::Menu<T> &m)
 {
 	if (!hasSelected(m.begin(), m.end()))
 		m.current().setSelected(true);
@@ -82,7 +85,15 @@ std::vector<Iterator> getSelectedOrCurrent(Iterator first, Iterator last, Iterat
 	return result;
 }
 
-template <typename T, typename F> void withUnfilteredMenu(NC::Menu<T> &m, F action)
+template <typename Iterator>
+void reverseSelectionHelper(Iterator first, Iterator last)
+{
+	for (; first != last; ++first)
+		first->setSelected(!first->isSelected());
+}
+
+template <typename T, typename F>
+void withUnfilteredMenu(NC::Menu<T> &m, F action)
 {
 	bool is_filtered = m.isFiltered();
 	m.showAll();
@@ -107,8 +118,8 @@ void withUnfilteredMenuReapplyFilter(NC::Menu<T> &m, F action)
 	}
 }
 
-template <typename T, typename F>
-void moveSelectedItemsUp(NC::Menu<T> &m, F swap_fun)
+template <typename F>
+void moveSelectedItemsUp(NC::Menu<MPD::Song> &m, F swap_fun)
 {
 	if (m.choice() > 0)
 		selectCurrentIfNoneSelected(m);
@@ -141,8 +152,8 @@ void moveSelectedItemsUp(NC::Menu<T> &m, F swap_fun)
 	}
 }
 
-template <typename T, typename F>
-void moveSelectedItemsDown(NC::Menu<T> &m, F swap_fun)
+template <typename F>
+void moveSelectedItemsDown(NC::Menu<MPD::Song> &m, F swap_fun)
 {
 	if (m.choice() < m.size()-1)
 		selectCurrentIfNoneSelected(m);
@@ -175,11 +186,9 @@ void moveSelectedItemsDown(NC::Menu<T> &m, F swap_fun)
 	}
 }
 
-template <typename T, typename F>
-void moveSelectedItemsTo(NC::Menu<T> &m, F move_fun)
+template <typename F>
+void moveSelectedItemsTo(NC::Menu<MPD::Song> &m, F move_fun)
 {
-	if (m.empty())
-		return;
 	auto cur_ptr = &m.current().value();
 	withUnfilteredMenu(m, [&]() {
 		// this is kinda shitty, but there is no other way to know
@@ -233,10 +242,65 @@ void moveSelectedItemsTo(NC::Menu<T> &m, F move_fun)
 	});
 }
 
-template <typename Iterator> void reverseSelectionHelper(Iterator first, Iterator last)
+template <typename F>
+bool deleteSelectedSongs(NC::Menu<MPD::Song> &m, F delete_fun)
 {
-	for (; first != last; ++first)
-		first->setSelected(!first->isSelected());
+	bool result = false;
+	selectCurrentIfNoneSelected(m);
+	// ok, this is tricky. we need to operate on whole playlist
+	// to get positions right, but at the same time we need to
+	// ignore all songs that are not filtered. we use the fact
+	// that both ranges share the same values, ie. we can compare
+	// pointers to check whether an item belongs to filtered range.
+	NC::Menu<MPD::Song>::Iterator begin;
+	NC::Menu<MPD::Song>::ReverseIterator real_begin, real_end;
+	withUnfilteredMenu(m, [&]() {
+		// obtain iterators for unfiltered range
+		begin = m.begin() + 1; // cancel reverse iterator's offset
+		real_begin = m.rbegin();
+		real_end = m.rend();
+	});
+	// get iterator to filtered range
+	auto cur_filtered = m.rbegin();
+	Mpd.StartCommandsList();
+	for (auto it = real_begin; it != real_end; ++it)
+	{
+		// current iterator belongs to filtered range, proceed
+		if (&*it == &*cur_filtered)
+		{
+			if (it->isSelected())
+			{
+				it->setSelected(false);
+				delete_fun(Mpd, it.base() - begin);
+			}
+			++cur_filtered;
+		}
+	}
+	if (Mpd.CommitCommandsList())
+		result = true;
+	return result;
+}
+
+template <typename F>
+bool cropPlaylist(NC::Menu<MPD::Song> &m, F delete_fun)
+{
+	reverseSelectionHelper(m.begin(), m.end());
+	return deleteSelectedSongs(m, delete_fun);
+}
+
+template <typename F, typename G>
+bool clearPlaylist(NC::Menu<MPD::Song> &m, F delete_fun, G clear_fun)
+{
+	bool result = false;
+	if (m.isFiltered())
+	{
+		for (auto it = m.begin(); it != m.end(); ++it)
+			it->setSelected(true);
+		result = deleteSelectedSongs(m, delete_fun);
+	}
+	else
+		result = clear_fun(Mpd);
+	return result;
 }
 
 template <typename Iterator> std::string getSharedDirectory(Iterator first, Iterator last)

src/mpdpp.cpp

@@ -537,7 +537,7 @@ void MPD::Connection::Shuffle()
 	}
 }
 
-bool MPD::Connection::ClearPlaylist()
+bool MPD::Connection::ClearMainPlaylist()
 {
 	if (!itsConnection)
 		return false;
@@ -997,7 +997,7 @@ bool MPD::Connection::DeleteID(unsigned id)
 	return result;
 }
 
-bool MPD::Connection::Delete(const std::string &playlist, unsigned pos)
+bool MPD::Connection::PlaylistDelete(const std::string &playlist, unsigned pos)
 {
 	if (!itsConnection)
 		return false;

src/mpdpp.h

@@ -125,7 +125,7 @@ namespace MPD
 			void Swap(unsigned, unsigned);
 			void Seek(unsigned);
 			void Shuffle();
-			bool ClearPlaylist();
+			bool ClearMainPlaylist();
 			
 			bool isPlaying() const { return GetState() > psStop; }
 			
@@ -184,7 +184,7 @@ namespace MPD
 			bool Add(const std::string &path);
 			bool Delete(unsigned);
 			bool DeleteID(unsigned);
-			bool Delete(const std::string &, unsigned);
+			bool PlaylistDelete(const std::string &, unsigned);
 			void StartCommandsList();
 			bool CommitCommandsList();