Playlist: use the Error library to return errors

src/Partition.hxx

@@ -55,10 +55,10 @@ struct Partition final : private PlayerListener, private MixerListener {
 		playlist.Clear(pc);
 	}
 
-	PlaylistResult AppendURI(const SongLoader &loader,
-				 const char *uri_utf8,
-				 unsigned *added_id=nullptr) {
-		return playlist.AppendURI(pc, loader, uri_utf8, added_id);
+	unsigned AppendURI(const SongLoader &loader,
+			   const char *uri_utf8,
+			   Error &error) {
+		return playlist.AppendURI(pc, loader, uri_utf8, error);
 	}
 
 	PlaylistResult DeletePosition(unsigned position) {

src/PlaylistSave.cxx

@@ -114,13 +114,16 @@ playlist_load_spl(struct playlist &playlist, PlayerControl &pc,
 		end_index = contents.size();
 
 	const SongLoader loader(nullptr, nullptr);
+	Error error2;
 
 	for (unsigned i = start_index; i < end_index; ++i) {
 		const auto &uri_utf8 = contents[i];
 
-		if ((playlist.AppendURI(pc, loader, uri_utf8.c_str())) != PlaylistResult::SUCCESS)
-			FormatError(playlist_domain,
-				    "can't add file \"%s\"", uri_utf8.c_str());
+		unsigned id = playlist.AppendURI(pc, loader, uri_utf8.c_str(),
+						 error2);
+		if (id == 0)
+			FormatError(error2, "can't add file \"%s\"",
+				    uri_utf8.c_str());
 	}
 
 	return true;

src/command/PlaylistCommands.cxx

@@ -66,16 +66,14 @@ handle_load(Client &client, int argc, char *argv[])
 	} else if (!check_range(client, &start_index, &end_index, argv[2]))
 		return CommandResult::ERROR;
 
-	const SongLoader loader(client);
-	const PlaylistResult result =
-		playlist_open_into_queue(argv[1],
-					 start_index, end_index,
-					 client.playlist,
-					 client.player_control, loader);
-	if (result != PlaylistResult::NO_SUCH_LIST)
-		return print_playlist_result(client, result);
-
 	Error error;
+	const SongLoader loader(client);
+	if (!playlist_open_into_queue(argv[1],
+				      start_index, end_index,
+				      client.playlist,
+				      client.player_control, loader, error))
+		return print_error(client, error);
+
 	if (playlist_load_spl(client.playlist, client.player_control,
 			      argv[1], start_index, end_index,
 			      error))

src/command/QueueCommands.cxx

@@ -64,8 +64,12 @@ handle_add(Client &client, gcc_unused int argc, char *argv[])
 
 	if (uri_has_scheme(uri) || PathTraitsUTF8::IsAbsolute(uri)) {
 		const SongLoader loader(client);
-		auto result = client.partition.AppendURI(loader, uri);
-		return print_playlist_result(client, result);
+		Error error;
+		unsigned id = client.partition.AppendURI(loader, uri, error);
+		if (id == 0)
+			return print_error(client, error);
+
+		return CommandResult::OK;
 	}
 
 #ifdef ENABLE_DATABASE
@@ -88,18 +92,16 @@ handle_addid(Client &client, int argc, char *argv[])
 		return CommandResult::ERROR;
 
 	const SongLoader loader(client);
-
-	unsigned added_id;
-	auto result = client.partition.AppendURI(loader, uri, &added_id);
-
-	if (result != PlaylistResult::SUCCESS)
-		return print_playlist_result(client, result);
+	Error error;
+	unsigned added_id = client.partition.AppendURI(loader, uri, error);
+	if (added_id == 0)
+		return print_error(client, error);
 
 	if (argc == 3) {
 		unsigned to;
 		if (!check_unsigned(client, &to, argv[2]))
 			return CommandResult::ERROR;
-		result = client.partition.MoveId(added_id, to);
+		PlaylistResult result = client.partition.MoveId(added_id, to);
 		if (result != PlaylistResult::SUCCESS) {
 			CommandResult ret =
 				print_playlist_result(client, result);

src/db/DatabaseQueue.cxx

@@ -23,7 +23,6 @@
 #include "Interface.hxx"
 #include "Partition.hxx"
 #include "Instance.hxx"
-#include "util/Error.hxx"
 #include "DetachedSong.hxx"
 
 #include <functional>
@@ -32,17 +31,12 @@ static bool
 AddToQueue(Partition &partition, const LightSong &song, Error &error)
 {
 	const Storage &storage = *partition.instance.storage;
-	PlaylistResult result =
+	unsigned id =
 		partition.playlist.AppendSong(partition.pc,
 					      DatabaseDetachSong(storage,
 								 song),
-					      nullptr);
-	if (result != PlaylistResult::SUCCESS) {
-		error.Set(playlist_domain, int(result), "Playlist error");
-		return false;
-	}
-
-	return true;
+					      error);
+	return id != 0;
 }
 
 bool

src/playlist/PlaylistQueue.cxx

@@ -27,16 +27,18 @@
 #include "thread/Mutex.hxx"
 #include "thread/Cond.hxx"
 #include "fs/Traits.hxx"
+#include "util/Error.hxx"
 
 #ifdef ENABLE_DATABASE
 #include "SongLoader.hxx"
 #endif
 
-PlaylistResult
+bool
 playlist_load_into_queue(const char *uri, SongEnumerator &e,
 			 unsigned start_index, unsigned end_index,
 			 playlist &dest, PlayerControl &pc,
-			 const SongLoader &loader)
+			 const SongLoader &loader,
+			 Error &error)
 {
 	const std::string base_uri = uri != nullptr
 		? PathTraitsUTF8::GetParent(uri)
@@ -58,20 +60,21 @@ playlist_load_into_queue(const char *uri, SongEnumerator &e,
 			continue;
 		}
 
-		PlaylistResult result = dest.AppendSong(pc, std::move(*song));
+		unsigned id = dest.AppendSong(pc, std::move(*song), error);
 		delete song;
-		if (result != PlaylistResult::SUCCESS)
-			return result;
+		if (id == 0)
+			return false;
 	}
 
-	return PlaylistResult::SUCCESS;
+	return true;
 }
 
-PlaylistResult
+bool
 playlist_open_into_queue(const char *uri,
 			 unsigned start_index, unsigned end_index,
 			 playlist &dest, PlayerControl &pc,
-			 const SongLoader &loader)
+			 const SongLoader &loader,
+			 Error &error)
 {
 	Mutex mutex;
 	Cond cond;
@@ -81,13 +84,16 @@ playlist_open_into_queue(const char *uri,
 					  loader.GetStorage(),
 #endif
 					  mutex, cond);
-	if (playlist == nullptr)
-		return PlaylistResult::NO_SUCH_LIST;
+	if (playlist == nullptr) {
+		error.Set(playlist_domain, int(PlaylistResult::NO_SUCH_LIST),
+			  "No such playlist");
+		return false;
+	}
 
-	PlaylistResult result =
+	bool result =
 		playlist_load_into_queue(uri, *playlist,
 					 start_index, end_index,
-					 dest, pc, loader);
+					 dest, pc, loader, error);
 	delete playlist;
 	return result;
 }

src/playlist/PlaylistQueue.hxx

@@ -26,6 +26,7 @@
 
 #include "PlaylistError.hxx"
 
+class Error;
 class SongLoader;
 class SongEnumerator;
 struct playlist;
@@ -40,21 +41,22 @@ struct PlayerControl;
  * @param start_index the index of the first song
  * @param end_index the index of the last song (excluding)
  */
-PlaylistResult
+bool
 playlist_load_into_queue(const char *uri, SongEnumerator &e,
 			 unsigned start_index, unsigned end_index,
 			 playlist &dest, PlayerControl &pc,
-			 const SongLoader &loader);
+			 const SongLoader &loader,
+			 Error &error);
 
 /**
  * Opens a playlist with a playlist plugin and append to the specified
  * play queue.
  */
-PlaylistResult
+bool
 playlist_open_into_queue(const char *uri,
 			 unsigned start_index, unsigned end_index,
 			 playlist &dest, PlayerControl &pc,
-			 const SongLoader &loader);
+			 const SongLoader &loader, Error &error);
 
 #endif
 

src/queue/Playlist.hxx

@@ -146,14 +146,20 @@ public:
 	void DatabaseModified(const Database &db);
 #endif
 
-	PlaylistResult AppendSong(PlayerControl &pc,
-				  DetachedSong &&song,
-				  unsigned *added_id=nullptr);
+	/**
+	 * @return the new song id or 0 on error
+	 */
+	unsigned AppendSong(PlayerControl &pc,
+			    DetachedSong &&song,
+			    Error &error);
 
-	PlaylistResult AppendURI(PlayerControl &pc,
-				 const SongLoader &loader,
-				 const char *uri_utf8,
-				 unsigned *added_id=nullptr);
+	/**
+	 * @return the new song id or 0 on error
+	 */
+	unsigned AppendURI(PlayerControl &pc,
+			   const SongLoader &loader,
+			   const char *uri_utf8,
+			   Error &error);
 
 protected:
 	void DeleteInternal(PlayerControl &pc,

src/queue/PlaylistEdit.cxx

@@ -32,7 +32,6 @@
 #include "DetachedSong.hxx"
 #include "SongLoader.hxx"
 #include "Idle.hxx"
-#include "Log.hxx"
 
 #include <stdlib.h>
 
@@ -55,14 +54,16 @@ playlist::Clear(PlayerControl &pc)
 	OnModified();
 }
 
-PlaylistResult
-playlist::AppendSong(PlayerControl &pc,
-		     DetachedSong &&song, unsigned *added_id)
+unsigned
+playlist::AppendSong(PlayerControl &pc, DetachedSong &&song, Error &error)
 {
 	unsigned id;
 
-	if (queue.IsFull())
-		return PlaylistResult::TOO_LARGE;
+	if (queue.IsFull()) {
+		error.Set(playlist_domain, int(PlaylistResult::TOO_LARGE),
+			  "Playlist is too large");
+		return 0;
+	}
 
 	const DetachedSong *const queued_song = GetQueuedSong();
 
@@ -84,30 +85,19 @@ playlist::AppendSong(PlayerControl &pc,
 	UpdateQueuedSong(pc, queued_song);
 	OnModified();
 
-	if (added_id)
-		*added_id = id;
-
-	return PlaylistResult::SUCCESS;
+	return id;
 }
 
-PlaylistResult
-playlist::AppendURI(PlayerControl &pc,
-		    const SongLoader &loader,
-		    const char *uri, unsigned *added_id)
+unsigned
+playlist::AppendURI(PlayerControl &pc, const SongLoader &loader,
+		    const char *uri,
+		    Error &error)
 {
-	FormatDebug(playlist_domain, "add to playlist: %s", uri);
-
-	Error error;
 	DetachedSong *song = loader.LoadSong(uri, error);
-	if (song == nullptr) {
-		// TODO: return the Error
-		LogError(error);
-		return error.IsDomain(playlist_domain)
-			? PlaylistResult(error.GetCode())
-			: PlaylistResult::NO_SUCH_SONG;
-	}
+	if (song == nullptr)
+		return 0;
 
-	PlaylistResult result = AppendSong(pc, std::move(*song), added_id);
+	unsigned result = AppendSong(pc, std::move(*song), error);
 	delete song;
 
 	return result;