Use VerificationNote more 'properly' in a fair few places.

4 files changed, 66 insertions, 25 deletions

diff --git a/src/dcp.cc b/src/dcp.cc
index 7f4540fa..e057f25f 100644
--- a/src/dcp.cc
+++ b/src/dcp.cc
@@ -200,7 +200,7 @@ DCP::read (list<dcp::VerificationNote>* notes, bool ignore_incorrect_picture_mxf
 
 		if (!boost::filesystem::exists(path)) {
 			if (notes) {
-				notes->push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::MISSING_ASSET));
+				notes->push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::MISSING_ASSET, path));
 			}
 			continue;
 		}
diff --git a/src/verify.cc b/src/verify.cc
index e20cafb3..9a2b0f70 100644
--- a/src/verify.cc
+++ b/src/verify.cc
@@ -352,10 +352,15 @@ dcp::verify (
 
 			list<XMLValidationError> errors = validate_xml (cpl->file().get(), xsd_dtd_directory);
 			BOOST_FOREACH (XMLValidationError i, errors) {
-				notes.push_back (VerificationNote(
-							 VerificationNote::VERIFY_ERROR, VerificationNote::Code::XML_VALIDATION_ERROR,
-							 String::compose("%1 (file %2, line %3)", i.message(), cpl->file()->string(), i.line())
-							 ));
+				notes.push_back (
+					VerificationNote(
+						VerificationNote::VERIFY_ERROR,
+						VerificationNote::Code::XML_VALIDATION_ERROR,
+						i.message(),
+						cpl->file().get(),
+						i.line()
+						)
+					);
 			}
 
 			/* Check that the CPL's hash corresponds to the PKL */
@@ -388,13 +393,17 @@ dcp::verify (
 						switch (r) {
 						case RESULT_BAD:
 							notes.push_back (
-									VerificationNote(
-										VerificationNote::VERIFY_ERROR, VerificationNote::PICTURE_HASH_INCORRECT, *reel->main_picture()->asset()->file()
-										)
-									);
+								VerificationNote(
+									VerificationNote::VERIFY_ERROR, VerificationNote::PICTURE_HASH_INCORRECT, *reel->main_picture()->asset()->file()
+									)
+								);
 							break;
 						case RESULT_CPL_PKL_DIFFER:
-							notes.push_back (VerificationNote(VerificationNote::VERIFY_ERROR, VerificationNote::PKL_CPL_PICTURE_HASHES_DISAGREE));
+							notes.push_back (
+								VerificationNote(
+									VerificationNote::VERIFY_ERROR, VerificationNote::PKL_CPL_PICTURE_HASHES_DISAGREE, *reel->main_picture()->asset()->file()
+									)
+								);
 							break;
 						default:
 							break;
@@ -407,13 +416,17 @@ dcp::verify (
 					switch (r) {
 					case RESULT_BAD:
 						notes.push_back (
-								VerificationNote(
-									VerificationNote::VERIFY_ERROR, VerificationNote::SOUND_HASH_INCORRECT, *reel->main_sound()->asset()->file()
-									)
-								);
+							VerificationNote(
+								VerificationNote::VERIFY_ERROR, VerificationNote::SOUND_HASH_INCORRECT, *reel->main_sound()->asset()->file()
+								)
+							);
 						break;
 					case RESULT_CPL_PKL_DIFFER:
-						notes.push_back (VerificationNote (VerificationNote::VERIFY_ERROR, VerificationNote::PKL_CPL_SOUND_HASHES_DISAGREE));
+						notes.push_back (
+							VerificationNote(
+								VerificationNote::VERIFY_ERROR, VerificationNote::PKL_CPL_SOUND_HASHES_DISAGREE, *reel->main_sound()->asset()->file()
+								)
+							);
 						break;
 					default:
 						break;
@@ -427,10 +440,15 @@ dcp::verify (
 
 			list<XMLValidationError> errors = validate_xml (pkl->file().get(), xsd_dtd_directory);
 			BOOST_FOREACH (XMLValidationError i, errors) {
-				notes.push_back (VerificationNote(
-							 VerificationNote::VERIFY_ERROR, VerificationNote::Code::XML_VALIDATION_ERROR,
-							 String::compose("%1 (file %2, line %3)", i.message(), pkl->file()->string(), i.line())
-							 ));
+				notes.push_back (
+					VerificationNote(
+						VerificationNote::VERIFY_ERROR,
+						VerificationNote::Code::XML_VALIDATION_ERROR,
+						i.message(),
+						pkl->file().get(),
+						i.line()
+						)
+					);
 			}
 		}
 	}
@@ -451,19 +469,19 @@ dcp::note_to_string (dcp::VerificationNote note)
 	case dcp::VerificationNote::PICTURE_HASH_INCORRECT:
 		return dcp::String::compose("The hash of the picture asset %1 does not agree with the PKL file", note.file()->filename());
 	case dcp::VerificationNote::PKL_CPL_PICTURE_HASHES_DISAGREE:
-		return "The PKL and CPL hashes disagree for a picture asset.";
+		return dcp::String::compose("The PKL and CPL hashes disagree for the picture asset %1", note.file()->filename());
 	case dcp::VerificationNote::SOUND_HASH_INCORRECT:
 		return dcp::String::compose("The hash of the sound asset %1 does not agree with the PKL file", note.file()->filename());
 	case dcp::VerificationNote::PKL_CPL_SOUND_HASHES_DISAGREE:
-		return "The PKL and CPL hashes disagree for a sound asset.";
+		return dcp::String::compose("The PKL and CPL hashes disagree for the sound asset %1", note.file()->filename());
 	case dcp::VerificationNote::EMPTY_ASSET_PATH:
 		return "The asset map contains an empty asset path.";
 	case dcp::VerificationNote::MISSING_ASSET:
-		return "The file for an asset in the asset map cannot be found.";
+		return String::compose("The file for an asset in the asset map cannot be found; missing file is %1.", note.file()->filename());
 	case dcp::VerificationNote::MISMATCHED_STANDARD:
 		return "The DCP contains both SMPTE and Interop parts.";
 	case dcp::VerificationNote::XML_VALIDATION_ERROR:
-		return "An XML file is badly formed.";
+		return String::compose("An XML file is badly formed: %1 (%2:%3)", note.note().get(), note.file()->filename(), note.line().get());
 	}
 
 	return "";
diff --git a/src/verify.h b/src/verify.h
index 9018ed46..ab9e9aac 100644
--- a/src/verify.h
+++ b/src/verify.h
@@ -71,7 +71,7 @@ public:
 		PKL_CPL_SOUND_HASHES_DISAGREE,
 		/** An assetmap's <Path> entry is empty */
 		EMPTY_ASSET_PATH,
-		/** An file mentioned in an assetmap cannot be found */
+		/** An file mentioned in an asset map cannot be found */
 		MISSING_ASSET,
 		/** The DCP contains both SMPTE and Interop-standard components */
 		MISMATCHED_STANDARD,
@@ -96,6 +96,21 @@ public:
 		, _file (file)
 	{}
 
+	VerificationNote (Type type, Code code, std::string note, boost::filesystem::path file)
+		: _type (type)
+		, _code (code)
+		, _note (note)
+		, _file (file)
+	{}
+
+	VerificationNote (Type type, Code code, std::string note, boost::filesystem::path file, uint64_t line)
+		: _type (type)
+		, _code (code)
+		, _note (note)
+		, _file (file)
+		, _line (line)
+	{}
+
 	Type type () const {
 		return _type;
 	}
@@ -112,11 +127,19 @@ public:
 		return _file;
 	}
 
+	boost::optional<uint64_t> line () const {
+		return _line;
+	}
+
 private:
 	Type _type;
 	Code _code;
+	/** Further information about the error, if applicable */
 	boost::optional<std::string> _note;
+	/** Path of file containing the error, if applicable */
 	boost::optional<boost::filesystem::path> _file;
+	/** Error line number within _file, if applicable */
+	uint64_t _line;
 };
 
 std::list<VerificationNote> verify (
diff --git a/test/verify_test.cc b/test/verify_test.cc
index f6f7ee43..f23e54c4 100644
--- a/test/verify_test.cc
+++ b/test/verify_test.cc
@@ -108,7 +108,7 @@ void
 dump_notes (list<dcp::VerificationNote> const & notes)
 {
 	BOOST_FOREACH (dcp::VerificationNote i, notes) {
-		std::cout << dcp::note_to_string(i) << " " << i.note().get_value_or("") << "\n";
+		std::cout << dcp::note_to_string(i) << "\n";
 	}
 }