From 61e2bcbefc99f89552450138f4ab36f1643b4aaa Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 11 Mar 2019 16:10:53 +0100 Subject: [PATCH 1/4] Make code more dry --- openstudiocore/src/utilities/core/PathHelpers.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/openstudiocore/src/utilities/core/PathHelpers.cpp b/openstudiocore/src/utilities/core/PathHelpers.cpp index 41726376669..c7693ce29e0 100644 --- a/openstudiocore/src/utilities/core/PathHelpers.cpp +++ b/openstudiocore/src/utilities/core/PathHelpers.cpp @@ -109,11 +109,7 @@ path setFileExtension(const path& p, { path result(p); path wext = toPath(ext); - std::string pext = openstudio::filesystem::extension(p); - if (!pext.empty()) { - // remove '.' from pext - pext = std::string(++pext.begin(),pext.end()); - } + std::string pext = getFileExtension(p); if (!pext.empty()) { if (pext != wext.string()) { if (warnOnMismatch) { From 67af30540b2b73ec3a75cdab116d60d697914b8b Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 11 Mar 2019 16:34:46 +0100 Subject: [PATCH 2/4] #72 add warning when trying to load a model that doesn't have OSC or OSM extension --- openstudiocore/src/utilities/idf/IdfFile.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/openstudiocore/src/utilities/idf/IdfFile.cpp b/openstudiocore/src/utilities/idf/IdfFile.cpp index 653c1a1d6b9..ceb2f03c550 100644 --- a/openstudiocore/src/utilities/idf/IdfFile.cpp +++ b/openstudiocore/src/utilities/idf/IdfFile.cpp @@ -381,9 +381,21 @@ OptionalIdfFile IdfFile::load(const path& p, path wp(p); if (iddFileType == IddFileType::OpenStudio) { + // can be Model or Component + std::string ext = getFileExtension(p); + if (! ( openstudio::istringEqual(ext, "osm") || + openstudio::istringEqual(ext, "osc")) ) { + LOG_FREE(Warn,"openstudio.setFileExtension","Path p, '" << toString(p) + << "', has an unexpected file extension. Was expecting 'osm' or 'osc'."); + } + + // This isn't issuing warnings because we pass false (we have to, since it can be either osm or osc) + // hence why we do check above wp = completePathToFile(wp,path(),modelFileExtension(),false); - if (wp.empty()) { wp = completePathToFile(wp,path(),componentFileExtension(),false); } + if (wp.empty()) { + wp = completePathToFile(wp,path(),componentFileExtension(),false); + } } else { wp = completePathToFile(wp,path(),"idf",true); From 948613672f9fe0cb74122fc9ee7a7a04ce62e6b1 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 11 Mar 2019 16:35:35 +0100 Subject: [PATCH 3/4] Fix for #72, return empty IdfFile (m_load returns false) if couldn't parse at least one valid IDF object. --- openstudiocore/src/utilities/idf/IdfFile.cpp | 23 ++++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/openstudiocore/src/utilities/idf/IdfFile.cpp b/openstudiocore/src/utilities/idf/IdfFile.cpp index ceb2f03c550..56b1c7614e2 100644 --- a/openstudiocore/src/utilities/idf/IdfFile.cpp +++ b/openstudiocore/src/utilities/idf/IdfFile.cpp @@ -624,9 +624,6 @@ bool IdfFile::m_load(std::istream& is, ProgressBar* progressBar, bool versionOnl else{ bool foundEndLine(false); - - // a valid Idf object to parse - ++objectNum; firstBlock = false; bool isVersion = false; @@ -682,16 +679,22 @@ bool IdfFile::m_load(std::istream& is, ProgressBar* progressBar, bool versionOnl } // construct the object - if (!versionOnly || isVersion) { + if (foundEndLine && (!versionOnly || isVersion)) { OptionalIdfObject object = IdfObject::load(text,*iddObject); if (!object) { LOG(Error,"Unable to construct IdfObject from text: " << std::endl << text << std::endl << "Throwing this object out and parsing the remainder of the file."); continue; + } else { + // a valid Idf object to parse + if (object->iddObject().type() != IddObjectType::Catchall) { + ++objectNum; + } + + // put it in the object list + addObject(*object); } - // put it in the object list - addObject(*object); } if (versionOnly && isVersion) { @@ -701,7 +704,13 @@ bool IdfFile::m_load(std::istream& is, ProgressBar* progressBar, bool versionOnl } } - return true; + // If we sucessfully parsed at least one object, we return true, otherwise false + if (objectNum > 0) { + return true; + } else { + LOG(Error, "Could not parse a single valid object in file."); + return false; + } } IddFileAndFactoryWrapper IdfFile::iddFileAndFactoryWrapper() const { From dc5bd72cb7aa42c2521d6bc8163abb5e5b6a2b69 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Mon, 11 Mar 2019 16:36:01 +0100 Subject: [PATCH 4/4] Modify a test, since now loading an empty string stream returns an empty OptionalIdfFile --- openstudiocore/src/utilities/idf/Test/IdfFile_GTest.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/openstudiocore/src/utilities/idf/Test/IdfFile_GTest.cpp b/openstudiocore/src/utilities/idf/Test/IdfFile_GTest.cpp index c7fe7e103cb..48c5d2bc90f 100644 --- a/openstudiocore/src/utilities/idf/Test/IdfFile_GTest.cpp +++ b/openstudiocore/src/utilities/idf/Test/IdfFile_GTest.cpp @@ -86,11 +86,7 @@ TEST_F(IdfFixture, IdfFile_BasicTests_LoadedFile) } TEST_F(IdfFixture, IdfFile_Header) { - std::stringstream ss; - OptionalIdfFile oFile = IdfFile::load(ss,IddFileType(IddFileType::EnergyPlus)); - ss.clear(); - ASSERT_TRUE(oFile); - IdfFile file = *oFile; + IdfFile file(IddFileType::EnergyPlus); std::string header = "! A one-line header. "; file.setHeader(header); EXPECT_EQ(header,file.header()); @@ -98,6 +94,7 @@ TEST_F(IdfFixture, IdfFile_Header) { header = "Not actually a header, should get ! pre-pended."; file.setHeader(header); EXPECT_NE(header,file.header()); + std::stringstream ss; ss << "! " << header; EXPECT_EQ(ss.str(),file.header());