With --set-all, read comments from stdin before processing tags (#29)

With --set-all, read comments from stdin before processing tags

src/cli.cc

@@ -52,12 +52,13 @@ static struct option getopt_options[] = {
 	{NULL, 0, 0, 0}
 };
 
-ot::status ot::parse_options(int argc, char** argv, ot::options& opt)
+ot::status ot::parse_options(int argc, char** argv, ot::options& opt, FILE* comments_input)
 {
 	static ot::encoding_converter to_utf8("", "UTF-8");
 	std::string utf8;
 	std::string::size_type equal;
 	ot::status rc;
+	bool set_all = false;
 	opt = {};
 	if (argc == 1)
 		return {st::bad_arguments, "No arguments specified. Use -h for help."};
@@ -100,7 +101,8 @@ ot::status ot::parse_options(int argc, char** argv, ot::options& opt)
 			opt.to_add.emplace_back(std::move(utf8));
 			break;
 		case 'S':
-			opt.set_all = true;
+			opt.delete_all = true;
+			set_all = true;
 			break;
 		case 'D':
 			opt.delete_all = true;
@@ -128,9 +130,19 @@ ot::status ot::parse_options(int argc, char** argv, ot::options& opt)
 		opt.path_out = opt.path_in;
 		opt.overwrite = true;
 	}
-	if (opt.path_in == "-" && opt.set_all)
+	if (opt.path_in == "-" && set_all)
 		return {st::bad_arguments,
 		        "Cannot use standard input as input file when --set-all is specified."};
+	if (set_all) {
+		// Read comments from stdin and prepend them to opt.to_add.
+		std::vector<std::string> comments;
+		auto rc = read_comments(comments_input, comments);
+		if (rc != st::ok)
+			return rc;
+		comments.reserve(comments.size() + opt.to_add.size());
+		std::move(opt.to_add.begin(), opt.to_add.end(), std::back_inserter(comments));
+		opt.to_add = std::move(comments);
+	}
 	return st::ok;
 }
 
@@ -177,7 +189,7 @@ void ot::print_comments(const std::list<std::string>& comments, FILE* output)
 		fputs("warning: Some tags contain control characters.\n", stderr);
 }
 
-ot::status ot::read_comments(FILE* input, std::list<std::string>& comments)
+ot::status ot::read_comments(FILE* input, std::vector<std::string>& comments)
 {
 	static ot::encoding_converter to_utf8("", "UTF-8");
 	comments.clear();
@@ -233,11 +245,7 @@ void ot::delete_comments(std::list<std::string>& comments, const std::string& se
 /** Apply the modifications requested by the user to the opustags packet. */
 static ot::status edit_tags(ot::opus_tags& tags, const ot::options& opt)
 {
-	if (opt.set_all) {
-		auto rc = ot::read_comments(stdin, tags.comments);
-		if (rc != ot::st::ok)
-			return rc;
-	} else if (opt.delete_all) {
+	if (opt.delete_all) {
 		tags.comments.clear();
 	} else for (const std::string& name : opt.to_delete) {
 		ot::delete_comments(tags.comments, name.c_str());

src/opustags.cc

@@ -17,7 +17,7 @@
 int main(int argc, char** argv) {
 	setlocale(LC_ALL, "");
 	ot::options opt;
-	ot::status rc = ot::parse_options(argc, argv, opt);
+	ot::status rc = ot::parse_options(argc, argv, opt, stdin);
 	if (rc == ot::st::ok)
 		rc = ot::run(opt);
 

src/opustags.h

@@ -410,7 +410,7 @@ struct options {
 	/**
 	 * Delete all the existing comments.
 	 *
-	 * Option: --delete-all
+	 * Option: --delete-all, --set-all
 	 */
 	bool delete_all = false;
 	/**
@@ -422,24 +422,15 @@ struct options {
 	 * Options: --add, --set, --set-all
 	 */
 	std::vector<std::string> to_add;
-	/**
-	 * Replace the previous comments by the ones supplied by the user.
-	 *
-	 * Read a list of comments from stdin and populate #to_add. Further comments may be added
-	 * with the --add option.
-	 *
-	 * Option: --set-all
-	 */
-	bool set_all = false;
 };
 
 /**
  * Parse the command-line arguments. Does not perform I/O related validations, but checks the
- * consistency of its arguments.
+ * consistency of its arguments. Comments are read if necessary from the given stream.
  *
  * On error, the state of the options structure is unspecified.
  */
-status parse_options(int argc, char** argv, options& opt);
+status parse_options(int argc, char** argv, options& opt, FILE* comments);
 
 /**
  * Print all the comments, separated by line breaks. Since a comment may contain line breaks, this
@@ -456,7 +447,7 @@ void print_comments(const std::list<std::string>& comments, FILE* output);
  *
  * The comments are converted from the system encoding to UTF-8, and returned as UTF-8.
  */
-status read_comments(FILE* input, std::list<std::string>& comments);
+status read_comments(FILE* input, std::vector<std::string>& comments);
 
 /**
  * Remove all comments matching the specified selector, which may either be a field name or a

t/cli.cc

@@ -7,7 +7,7 @@ using namespace std::literals::string_literals;
 
 void check_read_comments()
 {
-	std::list<std::string> comments;
+	std::vector<std::string> comments;
 	ot::status rc;
 	{
 		std::string txt = "TITLE=a b c\n\nARTIST=X\nArtist=Y\n"s;
@@ -39,13 +39,13 @@ void check_read_comments()
  * Wrap #ot::parse_options with a higher-level interface much more convenient for testing.
  * In practice, the argc/argv combo are enough though for the current state of opustags.
  */
-static ot::status parse_options(const std::vector<const char*>& args, ot::options& opt)
+static ot::status parse_options(const std::vector<const char*>& args, ot::options& opt, FILE *comments)
 {
 	int argc = args.size();
 	char* argv[argc];
 	for (size_t i = 0; i < argc; ++i)
 		argv[i] = strdup(args[i]);
-	ot::status rc = ot::parse_options(argc, argv, opt);
+	ot::status rc = ot::parse_options(argc, argv, opt, comments);
 	for (size_t i = 0; i < argc; ++i)
 		free(argv[i]);
 	return rc;
@@ -55,7 +55,9 @@ void check_good_arguments()
 {
 	auto parse = [](std::vector<const char*> args) {
 		ot::options opt;
-		ot::status rc = parse_options(args, opt);
+		std::string txt = "N=1\n"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		ot::status rc = parse_options(args, opt, input.get());
 		if (rc.code != ot::st::ok)
 			throw failure("unexpected option parsing error");
 		return opt;
@@ -73,21 +75,27 @@ void check_good_arguments()
 		throw failure("unexpected option parsing result for case #1");
 
 	opt = parse({"opustags", "-S", "x", "-S", "-a", "x=y z", "-i"});
-	if (opt.path_in != "x" || opt.path_out != "x" || !opt.set_all || !opt.overwrite ||
-	    opt.to_delete.size() != 0 || opt.to_add.size() != 1 || opt.to_add[0] != "x=y z")
+	if (opt.path_in != "x" || opt.path_out != "x" || !opt.overwrite ||
+	    opt.to_delete.size() != 0 || opt.to_add.size() != 2 || opt.to_add[0] != "N=1" ||
+	    opt.to_add[1] != "x=y z")
 		throw failure("unexpected option parsing result for case #2");
 }
 
 void check_bad_arguments()
 {
-	auto error_case = [](std::vector<const char*> args, const char* message, const std::string& name) {
+	auto error_code_case = [](std::vector<const char*> args, const char* message, ot::st error_code, const std::string& name) {
 		ot::options opt;
-		ot::status rc = parse_options(args, opt);
-		if (rc.code != ot::st::bad_arguments)
+		std::string txt = "N=1\nINVALID"s;
+		ot::file input = fmemopen((char*) txt.data(), txt.size(), "r");
+		ot::status rc = parse_options(args, opt, input.get());
+		if (rc.code != error_code)
 			throw failure("bad error code for case " + name);
 		if (rc.message != message)
 			throw failure("bad error message for case " + name + ", got: " + rc.message);
 	};
+	auto error_case = [&error_code_case](std::vector<const char*> args, const char* message, const std::string& name) {
+		 error_code_case(args, message, ot::st::bad_arguments, name);
+	};
 	error_case({"opustags"}, "No arguments specified. Use -h for help.", "no arguments");
 	error_case({"opustags", "--output", ""}, "Output file path cannot be empty.", "empty output path");
 	error_case({"opustags", "-a", "X"}, "Comment does not contain an equal sign: X.", "bad comment for -a");
@@ -106,6 +114,7 @@ void check_bad_arguments()
 	error_case({"opustags", "-i", "-"}, "Cannot modify standard input in place.", "write stdin in-place");
 	error_case({"opustags", "-o", "x", "--output", "y", "z"},
 	           "Cannot specify --output more than once.", "double output");
+	error_code_case({"opustags", "-S", "x"}, "Malformed tag: INVALID", ot::st::error, "attempt to read invalid argument with -S");
 }
 
 static void check_delete_comments()