From f85eb05c35ce4509c78d4a990a130aa5d50ff078 Mon Sep 17 00:00:00 2001 From: Jonathan Laroche Date: Tue, 10 Dec 2024 14:35:08 -0500 Subject: [PATCH 1/9] Adding support for commas in active views list --- include/OpenColorIO/OpenColorAppHelpers.h | 4 + include/OpenColorIO/OpenColorIO.h | 10 ++ src/OpenColorIO/Config.cpp | 123 ++++++++++++++++++ src/OpenColorIO/OCIOYaml.cpp | 19 ++- src/OpenColorIO/ParseUtils.cpp | 97 ++++++++++++-- .../mergeconfigs/MergeConfigsHelpers.cpp | 20 +++ .../apphelpers/mergeconfigs/OCIOMYaml.cpp | 19 ++- 7 files changed, 271 insertions(+), 21 deletions(-) diff --git a/include/OpenColorIO/OpenColorAppHelpers.h b/include/OpenColorIO/OpenColorAppHelpers.h index 9bb40e42c5..854eaf81d0 100644 --- a/include/OpenColorIO/OpenColorAppHelpers.h +++ b/include/OpenColorIO/OpenColorAppHelpers.h @@ -666,9 +666,13 @@ class OCIOEXPORT ConfigMergingParameters void setActiveDisplays(const char * displays); const char * getActiveDisplays() const; + int getNumActiveDisplays() const; + const char * getActiveDisplay(int index) const; void setActiveViews(const char * views); const char * getActiveViews() const; + int getNumActiveViews() const; + const char * getActiveView(int index) const; void setInactiveColorspaces(const char * colorspaces); const char * getInactiveColorSpaces() const; diff --git a/include/OpenColorIO/OpenColorIO.h b/include/OpenColorIO/OpenColorIO.h index 982d0facd2..5af3fb4de0 100644 --- a/include/OpenColorIO/OpenColorIO.h +++ b/include/OpenColorIO/OpenColorIO.h @@ -1086,6 +1086,11 @@ class OCIOEXPORT Config */ void setActiveDisplays(const char * displays); const char * getActiveDisplays() const; + void addActiveDisplay(const char * view); + void removeActiveDisplay(const char * view); + void clearActiveDisplays(); + const char * getActiveDisplay(int index) const; + int getNumActiveDisplays() const; /** * \brief @@ -1104,6 +1109,11 @@ class OCIOEXPORT Config */ void setActiveViews(const char * views); const char * getActiveViews() const; + void addActiveView(const char * view); + void removeActiveView(const char * view); + void clearActiveViews(); + const char * getActiveView(int index) const; + int getNumActiveViews() const; /// Get all displays in the config, ignoring the active_displays list. int getNumDisplaysAll() const noexcept; diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index f244ae435e..5511699d22 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -4005,6 +4005,69 @@ const char * Config::getActiveDisplays() const return getImpl()->m_activeDisplaysStr.c_str(); } +void Config::removeActiveDisplay(const char * display) +{ + if( !display || !display[0] ) + { + throw Exception("Active display could not be removed from config, display name has to be a " + "non-empty name."); + } + + auto it = std::find(getImpl()->m_activeDisplays.begin(), + getImpl()->m_activeDisplays.end(), display); + + if(it!=getImpl()->m_activeDisplays.end()) + { + getImpl()->m_activeDisplays.erase(it); + } + + getImpl()->m_displayCache.clear(); + + AutoMutex lock(getImpl()->m_cacheidMutex); + getImpl()->resetCacheIDs(); +} + +void Config::clearActiveDisplays() +{ + getImpl()->m_activeDisplays.clear(); + + getImpl()->m_displayCache.clear(); + + AutoMutex lock(getImpl()->m_cacheidMutex); + getImpl()->resetCacheIDs(); +} + +const char * Config::getActiveDisplay( int index ) const +{ + if( index<0 || index >= static_cast(getImpl()->m_activeDisplays.size())) + { + return nullptr; + } + + return getImpl()->m_activeDisplays[index].c_str(); +} + +int Config::getNumActiveDisplays() const +{ + return static_cast(getImpl()->m_activeDisplays.size()); +} + +void Config::addActiveDisplay(const char * view) +{ + if( !view || !view[0] ) + { + throw Exception("Active view could not be added to config, view name has to be a " + "non-empty name."); + } + + getImpl()->m_activeDisplays.push_back(view); + + getImpl()->m_displayCache.clear(); + + AutoMutex lock(getImpl()->m_cacheidMutex); + getImpl()->resetCacheIDs(); +} + void Config::setActiveViews(const char * views) { getImpl()->m_activeViews.clear(); @@ -4022,6 +4085,66 @@ const char * Config::getActiveViews() const return getImpl()->m_activeViewsStr.c_str(); } +void Config::addActiveView(const char * view) +{ + if( !view || !view[0] ) + { + throw Exception("Active view could not be added to config, view name has to be a " + "non-empty name."); + } + + getImpl()->m_activeViews.push_back(view); + + getImpl()->m_displayCache.clear(); + AutoMutex lock(getImpl()->m_cacheidMutex); + getImpl()->resetCacheIDs(); +} + +void Config::removeActiveView(const char * view) +{ + if( !view || !view[0] ) + { + throw Exception("Active view could not be removed from config, view name has to be a " + "non-empty name."); + } + + auto it = std::find(getImpl()->m_activeViews.begin(), + getImpl()->m_activeViews.end(), view); + + if(it!=getImpl()->m_activeViews.end()) + { + getImpl()->m_activeViews.erase(it); + } + + getImpl()->m_displayCache.clear(); + AutoMutex lock(getImpl()->m_cacheidMutex); + getImpl()->resetCacheIDs(); +} + +void Config::clearActiveViews() +{ + getImpl()->m_activeViews.clear(); + + getImpl()->m_displayCache.clear(); + AutoMutex lock(getImpl()->m_cacheidMutex); + getImpl()->resetCacheIDs(); +} + +const char * Config::getActiveView( int index ) const +{ + if( index<0 || index >= static_cast(getImpl()->m_activeViews.size())) + { + return nullptr; + } + + return getImpl()->m_activeViews[index].c_str(); +} + +int Config::getNumActiveViews() const +{ + return static_cast(getImpl()->m_activeViews.size()); +} + int Config::getNumDisplaysAll() const noexcept { return static_cast(getImpl()->m_displays.size()); diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index b1cee18982..6805b325ba 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -4992,13 +4992,24 @@ inline void save(YAML::Emitter & out, const Config & config) out << YAML::Newline; out << YAML::Key << "active_displays"; StringUtils::StringVec active_displays; - if(config.getActiveDisplays() != NULL && strlen(config.getActiveDisplays()) > 0) - active_displays = SplitStringEnvStyle(config.getActiveDisplays()); + int nDisplays = config.getNumActiveDisplays(); + active_displays.reserve( nDisplays ); + for (int i = 0; i < nDisplays; i++) + { + active_displays.push_back(config.getActiveDisplay(i)); + } + out << YAML::Value << YAML::Flow << active_displays; + + out << YAML::Key << "active_views"; StringUtils::StringVec active_views; - if(config.getActiveViews() != NULL && strlen(config.getActiveViews()) > 0) - active_views = SplitStringEnvStyle(config.getActiveViews()); + int nViews = config.getNumActiveViews(); + active_views.reserve( nViews ); + for (int i = 0; i < nViews; i++) + { + active_views.push_back(config.getActiveView(i)); + } out << YAML::Value << YAML::Flow << active_views; const std::string inactiveCSs = config.getInactiveColorSpaces(); diff --git a/src/OpenColorIO/ParseUtils.cpp b/src/OpenColorIO/ParseUtils.cpp index a8a8a7a3c1..a3346ce0ad 100644 --- a/src/OpenColorIO/ParseUtils.cpp +++ b/src/OpenColorIO/ParseUtils.cpp @@ -690,33 +690,104 @@ bool StrEqualsCaseIgnore(const std::string & a, const std::string & b) return 0 == Platform::Strcasecmp(a.c_str(), b.c_str()); } +// Find the end of a name from a list contained in a string. +// The element of the list are separeted by ":" or ",". +// The name can be surrounded by quotes to enable name including theses symbols. +static size_t findEndOfName(const std::string & s, size_t start) +{ + size_t currentPos = start; + size_t nameEndPos = currentPos; + bool isEndFound = false; + + while( !isEndFound ) + { + nameEndPos = s.find_first_of("\",:", currentPos); + if(nameEndPos == std::string::npos) + { + // We reached the end of the list + nameEndPos = s.size() - 1; + isEndFound = true; + } + else if( s[nameEndPos] == '"' ) + { + // We found a quote, we need to find the next one + nameEndPos = s.find_first_of("\"", nameEndPos + 1); + if(nameEndPos == std::string::npos) + { + // We reached the end of the list instead + nameEndPos = s.size() - 1; + isEndFound = true; + } + else + { + // We found the second quote, + // we need to continue the search for a symbol separating the elements (: or ,) + currentPos = nameEndPos + 1; + } + } + else if( s[nameEndPos] == ',' || + s[nameEndPos] == ':' ) + { + // We found a symbol separating the elements, we stop here + nameEndPos--; + isEndFound = true; + } + } + + return nameEndPos; +} + StringUtils::StringVec SplitStringEnvStyle(const std::string & str) { StringUtils::StringVec outputvec; const std::string s = StringUtils::Trim(str); - if (StringUtils::Find(s, ",") != std::string::npos) - { - outputvec = StringUtils::Split(s, ','); - } - else if (StringUtils::Find(s, ":") != std::string::npos) - { - outputvec = StringUtils::Split(s, ':'); - } - else + size_t currentPos = 0; + + + while( s.size() > 0 && + currentPos < s.size() - 1 ) { - outputvec.push_back(s); + size_t nameEndPos = findEndOfName(s, currentPos); + outputvec.push_back(s.substr(currentPos, nameEndPos - currentPos + 1)); + currentPos = nameEndPos + 2; } - for (auto & val : outputvec) + for ( auto & val : outputvec ) { - val = StringUtils::Trim(val); + const std::string trimmedValue = StringUtils::Trim(val); + + // If the trimmed value is surrounded by quotes, we remove them + if( trimmedValue.size() > 1 && + trimmedValue[0] == '"' && + trimmedValue[trimmedValue.size() - 1] == '"' ) + { + val = trimmedValue.substr(1, trimmedValue.size() - 2); + } + else + { + val = trimmedValue; + } } + return outputvec; } std::string JoinStringEnvStyle(const StringUtils::StringVec & outputvec) { - return StringUtils::Join(outputvec, ','); + std::string result; + for( const auto & val : outputvec ) + { + if( val.find_first_of(',') != std::string::npos ) + { + result += "\"" + val + "\", "; + } + else + { + result += val + ", "; + } + } + + return result; } // Return a vector of strings that are both in vec1 and vec2. diff --git a/src/OpenColorIO/apphelpers/mergeconfigs/MergeConfigsHelpers.cpp b/src/OpenColorIO/apphelpers/mergeconfigs/MergeConfigsHelpers.cpp index 0e5f770f04..1ea158254e 100644 --- a/src/OpenColorIO/apphelpers/mergeconfigs/MergeConfigsHelpers.cpp +++ b/src/OpenColorIO/apphelpers/mergeconfigs/MergeConfigsHelpers.cpp @@ -153,6 +153,16 @@ const char * ConfigMergingParameters::getActiveDisplays() const return getImpl()->m_overrideCfg->getActiveDisplays(); } +int ConfigMergingParameters::getNumActiveDisplays() const +{ + return getImpl()->m_overrideCfg->getNumActiveDisplays(); +} + +const char * ConfigMergingParameters::getActiveDisplay(int index) const +{ + return getImpl()->m_overrideCfg->getActiveDisplay(index); +} + void ConfigMergingParameters::setActiveViews(const char * views) { getImpl()->m_overrideCfg->setActiveViews(views); @@ -163,6 +173,16 @@ const char * ConfigMergingParameters::getActiveViews() const return getImpl()->m_overrideCfg->getActiveViews(); } +int ConfigMergingParameters::getNumActiveViews() const +{ + return getImpl()->m_overrideCfg->getNumActiveViews(); +} + +const char * ConfigMergingParameters::getActiveView(int index) const +{ + return getImpl()->m_overrideCfg->getActiveView(index); +} + void ConfigMergingParameters::setInactiveColorspaces(const char * colorspaces) { getImpl()->m_overrideCfg->setInactiveColorSpaces(colorspaces); diff --git a/src/OpenColorIO/apphelpers/mergeconfigs/OCIOMYaml.cpp b/src/OpenColorIO/apphelpers/mergeconfigs/OCIOMYaml.cpp index 76b0e3be74..3b65170546 100644 --- a/src/OpenColorIO/apphelpers/mergeconfigs/OCIOMYaml.cpp +++ b/src/OpenColorIO/apphelpers/mergeconfigs/OCIOMYaml.cpp @@ -548,15 +548,26 @@ inline void save(YAML::Emitter & out, const ConfigMerger & merger) out << YAML::Key << "active_displays"; StringUtils::StringVec active_displays; - if (p->getActiveDisplays() != NULL && strlen(p->getActiveDisplays()) > 0) - active_displays = SplitStringEnvStyle(p->getActiveDisplays()); + int nDisplays = p->getNumActiveDisplays(); + active_displays.reserve( nDisplays ); + for (int i = 0; i < nDisplays; i++) + { + active_displays.push_back(p->getActiveDisplay(i)); + } + + out << YAML::Value << YAML::Flow << active_displays; out << YAML::Newline; out << YAML::Key << "active_views"; StringUtils::StringVec active_views; - if (p->getActiveViews() != NULL && strlen(p->getActiveViews()) > 0) - active_views = SplitStringEnvStyle(p->getActiveViews()); + int nViews = p->getNumActiveViews(); + active_views.reserve( nViews ); + for (int i = 0; i < nViews; i++) + { + active_views.push_back(p->getActiveView(i)); + } + out << YAML::Value << YAML::Flow << active_views; out << YAML::Key << "inactive_colorspaces"; From 6c4178077bbf917b60b633ca38a5fc5b94284357 Mon Sep 17 00:00:00 2001 From: Jonathan Laroche Date: Wed, 11 Dec 2024 11:47:29 -0500 Subject: [PATCH 2/9] Review fixes: -Adding test cases and some parsing fixes -Fix for join -Fix for bug in split string and move order of functions --- src/OpenColorIO/Config.cpp | 66 +++++++++++++++++--------- src/OpenColorIO/ParseUtils.cpp | 72 ++++++++++++++++++++-------- tests/cpu/ParseUtils_tests.cpp | 86 ++++++++++++++++++++++++++++++++-- 3 files changed, 179 insertions(+), 45 deletions(-) diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 5511699d22..2b4db90f7a 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -3411,7 +3411,7 @@ void Config::removeSharedView(const char * view) { std::ostringstream os; os << "Shared view could not be removed from config. A shared view named '" - << view << "' could be be found."; + << view << "' could not be found."; throw Exception(os.str().c_str()); } } @@ -4005,20 +4005,54 @@ const char * Config::getActiveDisplays() const return getImpl()->m_activeDisplaysStr.c_str(); } +void Config::addActiveDisplay(const char * view) +{ + if( !view || !view[0] ) + { + throw Exception("Active display could not be added to config, display name has to be a " + "non-empty name."); + } + + auto it = std::find(getImpl()->m_activeDisplays.begin(), + getImpl()->m_activeDisplays.end(), view); + + if( it != getImpl()->m_activeDisplays.end() ) + { + std::ostringstream os; + os << "Active display could not be added to config. An active display named '" + << view << "' already exists."; + throw Exception(os.str().c_str()); + } + + getImpl()->m_activeDisplays.push_back(view); + + getImpl()->m_displayCache.clear(); + + AutoMutex lock(getImpl()->m_cacheidMutex); + getImpl()->resetCacheIDs(); +} + void Config::removeActiveDisplay(const char * display) { if( !display || !display[0] ) { - throw Exception("Active display could not be removed from config, display name has to be a " - "non-empty name."); + throw Exception("Active display could not be removed from config, display name has to be a " + "non-empty name."); } auto it = std::find(getImpl()->m_activeDisplays.begin(), getImpl()->m_activeDisplays.end(), display); - if(it!=getImpl()->m_activeDisplays.end()) + if( it != getImpl()->m_activeDisplays.end() ) + { + getImpl()->m_activeDisplays.erase(it); + } + else { - getImpl()->m_activeDisplays.erase(it); + std::ostringstream os; + os << "Active display could not be removed from config. An active display named '" + << display << "' could not be found."; + throw Exception(os.str().c_str()); } getImpl()->m_displayCache.clear(); @@ -4039,7 +4073,8 @@ void Config::clearActiveDisplays() const char * Config::getActiveDisplay( int index ) const { - if( index<0 || index >= static_cast(getImpl()->m_activeDisplays.size())) + if( index<0 || + index >= static_cast(getImpl()->m_activeDisplays.size())) { return nullptr; } @@ -4052,22 +4087,6 @@ int Config::getNumActiveDisplays() const return static_cast(getImpl()->m_activeDisplays.size()); } -void Config::addActiveDisplay(const char * view) -{ - if( !view || !view[0] ) - { - throw Exception("Active view could not be added to config, view name has to be a " - "non-empty name."); - } - - getImpl()->m_activeDisplays.push_back(view); - - getImpl()->m_displayCache.clear(); - - AutoMutex lock(getImpl()->m_cacheidMutex); - getImpl()->resetCacheIDs(); -} - void Config::setActiveViews(const char * views) { getImpl()->m_activeViews.clear(); @@ -4132,7 +4151,8 @@ void Config::clearActiveViews() const char * Config::getActiveView( int index ) const { - if( index<0 || index >= static_cast(getImpl()->m_activeViews.size())) + if( index<0 || + index >= static_cast(getImpl()->m_activeViews.size())) { return nullptr; } diff --git a/src/OpenColorIO/ParseUtils.cpp b/src/OpenColorIO/ParseUtils.cpp index a3346ce0ad..64d459a8fe 100644 --- a/src/OpenColorIO/ParseUtils.cpp +++ b/src/OpenColorIO/ParseUtils.cpp @@ -693,29 +693,29 @@ bool StrEqualsCaseIgnore(const std::string & a, const std::string & b) // Find the end of a name from a list contained in a string. // The element of the list are separeted by ":" or ",". // The name can be surrounded by quotes to enable name including theses symbols. -static size_t findEndOfName(const std::string & s, size_t start) +static int findEndOfName(const std::string & s, size_t start) { - size_t currentPos = start; - size_t nameEndPos = currentPos; + int currentPos = start; + int nameEndPos = currentPos; bool isEndFound = false; while( !isEndFound ) { nameEndPos = s.find_first_of("\",:", currentPos); - if(nameEndPos == std::string::npos) + if(nameEndPos == (int)std::string::npos) { // We reached the end of the list - nameEndPos = s.size() - 1; + nameEndPos = s.size(); isEndFound = true; } else if( s[nameEndPos] == '"' ) { // We found a quote, we need to find the next one nameEndPos = s.find_first_of("\"", nameEndPos + 1); - if(nameEndPos == std::string::npos) + if(nameEndPos == (int)std::string::npos) { // We reached the end of the list instead - nameEndPos = s.size() - 1; + nameEndPos = s.size(); isEndFound = true; } else @@ -729,7 +729,6 @@ static size_t findEndOfName(const std::string & s, size_t start) s[nameEndPos] == ':' ) { // We found a symbol separating the elements, we stop here - nameEndPos--; isEndFound = true; } } @@ -739,17 +738,28 @@ static size_t findEndOfName(const std::string & s, size_t start) StringUtils::StringVec SplitStringEnvStyle(const std::string & str) { - StringUtils::StringVec outputvec; const std::string s = StringUtils::Trim(str); - size_t currentPos = 0; + if( s.size() == 0 ) + { + return {}; + } - + StringUtils::StringVec outputvec; + int currentPos = 0; while( s.size() > 0 && - currentPos < s.size() - 1 ) + currentPos <= (int) s.size() ) { - size_t nameEndPos = findEndOfName(s, currentPos); - outputvec.push_back(s.substr(currentPos, nameEndPos - currentPos + 1)); - currentPos = nameEndPos + 2; + int nameEndPos = findEndOfName(s, currentPos); + if(nameEndPos > currentPos) + { + outputvec.push_back(s.substr(currentPos, nameEndPos - currentPos)); + currentPos = nameEndPos + 1; + } + else + { + outputvec.push_back(""); + currentPos += 1; + } } for ( auto & val : outputvec ) @@ -775,15 +785,39 @@ StringUtils::StringVec SplitStringEnvStyle(const std::string & str) std::string JoinStringEnvStyle(const StringUtils::StringVec & outputvec) { std::string result; - for( const auto & val : outputvec ) + const int nElement = static_cast(outputvec.size()); + if( nElement == 0 ) + { + return ""; + } + + // We check if the value contains a symbol that could be interpreted as a separator + // and if it is not already surrounded by quotes + const std::string& firstValue = outputvec[0]; + if( firstValue.find_first_of(",:") != std::string::npos && + firstValue.size() > 1 && + firstValue[0] != '"' && + firstValue[firstValue.size() - 1] != '"' ) + { + result += "\"" + firstValue + "\""; + } + else + { + result += firstValue; + } + + for( int i = 1; i < nElement; ++i ) { - if( val.find_first_of(',') != std::string::npos ) + if( outputvec[i].find_first_of(",:") != std::string::npos && + outputvec[i].size() > 1 && + outputvec[i][0] != '"' && + outputvec[i][outputvec[i].size() - 1] != '"' ) { - result += "\"" + val + "\", "; + result += ", \"" + outputvec[i] + "\""; } else { - result += val + ", "; + result += ", " + outputvec[i]; } } diff --git a/tests/cpu/ParseUtils_tests.cpp b/tests/cpu/ParseUtils_tests.cpp index 42f20360c8..2af2ce84b0 100644 --- a/tests/cpu/ParseUtils_tests.cpp +++ b/tests/cpu/ParseUtils_tests.cpp @@ -395,6 +395,7 @@ OCIO_ADD_TEST(ParseUtils, split_string_env_style) OCIO_CHECK_EQUAL("a", outputvec[2]); OCIO_CHECK_EQUAL("test", outputvec[3]); outputvec.clear(); + outputvec = OCIO::SplitStringEnvStyle(" This : is : a: test "); OCIO_CHECK_EQUAL(4, outputvec.size()); OCIO_CHECK_EQUAL("This", outputvec[0]); @@ -402,6 +403,7 @@ OCIO_ADD_TEST(ParseUtils, split_string_env_style) OCIO_CHECK_EQUAL("a", outputvec[2]); OCIO_CHECK_EQUAL("test", outputvec[3]); outputvec.clear(); + outputvec = OCIO::SplitStringEnvStyle(" This , is , a, test "); OCIO_CHECK_EQUAL(4, outputvec.size()); OCIO_CHECK_EQUAL("This", outputvec[0]); @@ -409,16 +411,94 @@ OCIO_ADD_TEST(ParseUtils, split_string_env_style) OCIO_CHECK_EQUAL("a", outputvec[2]); OCIO_CHECK_EQUAL("test", outputvec[3]); outputvec.clear(); + outputvec = OCIO::SplitStringEnvStyle("This:is , a:test "); - OCIO_CHECK_EQUAL(2, outputvec.size()); - OCIO_CHECK_EQUAL("This:is", outputvec[0]); - OCIO_CHECK_EQUAL("a:test", outputvec[1]); + OCIO_CHECK_EQUAL(4, outputvec.size()); + OCIO_CHECK_EQUAL("This", outputvec[0]); + OCIO_CHECK_EQUAL("is", outputvec[1]); + OCIO_CHECK_EQUAL("a", outputvec[2]); + OCIO_CHECK_EQUAL("test", outputvec[3]); outputvec.clear(); + outputvec = OCIO::SplitStringEnvStyle(",,"); OCIO_CHECK_EQUAL(3, outputvec.size()); OCIO_CHECK_EQUAL("", outputvec[0]); OCIO_CHECK_EQUAL("", outputvec[1]); OCIO_CHECK_EQUAL("", outputvec[2]); + + outputvec = OCIO::SplitStringEnvStyle(" \"This : is \": a: test "); + OCIO_CHECK_EQUAL(3, outputvec.size()); + OCIO_CHECK_EQUAL("This : is ", outputvec[0]); + OCIO_CHECK_EQUAL("a", outputvec[1]); + OCIO_CHECK_EQUAL("test", outputvec[2]); + + outputvec = OCIO::SplitStringEnvStyle(" This : is \": a: test "); + OCIO_CHECK_EQUAL(2, outputvec.size()); + OCIO_CHECK_EQUAL("This", outputvec[0]); + OCIO_CHECK_EQUAL("is \": a: test", outputvec[1]); + + outputvec = OCIO::SplitStringEnvStyle(" This : is \": a: test \""); + OCIO_CHECK_EQUAL(2, outputvec.size()); + OCIO_CHECK_EQUAL("This", outputvec[0]); + OCIO_CHECK_EQUAL("is \": a: test \"" , outputvec[1]); + + outputvec = OCIO::SplitStringEnvStyle(" \"This : is \", a, test "); + OCIO_CHECK_EQUAL(3, outputvec.size()); + OCIO_CHECK_EQUAL("This : is ", outputvec[0]); + OCIO_CHECK_EQUAL("a", outputvec[1]); + OCIO_CHECK_EQUAL("test", outputvec[2]); + + outputvec = OCIO::SplitStringEnvStyle(" \"This , is \": a: test "); + OCIO_CHECK_EQUAL(3, outputvec.size()); + OCIO_CHECK_EQUAL("This , is ", outputvec[0]); + OCIO_CHECK_EQUAL("a", outputvec[1]); + OCIO_CHECK_EQUAL("test", outputvec[2]); + + outputvec = OCIO::SplitStringEnvStyle(" \"This , is \": a, test "); + OCIO_CHECK_EQUAL(3, outputvec.size()); + OCIO_CHECK_EQUAL("This , is ", outputvec[0]); + OCIO_CHECK_EQUAL("a", outputvec[1]); + OCIO_CHECK_EQUAL("test", outputvec[2]); + + outputvec = OCIO::SplitStringEnvStyle(" This : is \": a: test "); + OCIO_CHECK_EQUAL(2, outputvec.size()); + OCIO_CHECK_EQUAL("This", outputvec[0]); + OCIO_CHECK_EQUAL("is \": a: test", outputvec[1]); + + outputvec = OCIO::SplitStringEnvStyle(" This : is \": a: test \""); + OCIO_CHECK_EQUAL(2, outputvec.size()); + OCIO_CHECK_EQUAL("This", outputvec[0]); + OCIO_CHECK_EQUAL("is \": a: test \"" , outputvec[1]); + + outputvec = OCIO::SplitStringEnvStyle(" This : is : a: test \""); + OCIO_CHECK_EQUAL(4, outputvec.size()); + OCIO_CHECK_EQUAL("This", outputvec[0]); + OCIO_CHECK_EQUAL("is", outputvec[1]); + OCIO_CHECK_EQUAL("a", outputvec[2]); + OCIO_CHECK_EQUAL("test \"", outputvec[3]); +} + +OCIO_ADD_TEST(ParseUtils, join_string_env_style) +{ + StringUtils::StringVec outputvec {"This", "is", "a", "test"}; + + OCIO_CHECK_EQUAL( "This, is, a, test", OCIO::JoinStringEnvStyle(outputvec) ); + outputvec.clear(); + + outputvec = { "This:is", "a:test" }; + OCIO_CHECK_EQUAL( "\"This:is\", \"a:test\"", OCIO::JoinStringEnvStyle(outputvec) ); + outputvec.clear(); + + outputvec = { "", "", "" }; + OCIO_CHECK_EQUAL( ", , ", OCIO::JoinStringEnvStyle(outputvec) ); + outputvec.clear(); + + outputvec = { "This : is", "a: test" }; + OCIO_CHECK_EQUAL( "\"This : is\", \"a: test\"", OCIO::JoinStringEnvStyle(outputvec) ); + outputvec.clear(); + + outputvec = { "This", "is \": a: test" }; + OCIO_CHECK_EQUAL( "This, \"is \": a: test\"", OCIO::JoinStringEnvStyle(outputvec) ); } OCIO_ADD_TEST(ParseUtils, intersect_string_vecs_case_ignore) From d2ebf42252cb2a53f4944c1cf8bdc6548315c251 Mon Sep 17 00:00:00 2001 From: Jonathan Laroche Date: Wed, 11 Dec 2024 20:13:33 -0500 Subject: [PATCH 3/9] Fix for look parse test failing, added a test case for SplitStringEnvStyle and JoinStringEnvStyle. --- src/OpenColorIO/LookParse.cpp | 6 ++++++ tests/cpu/ParseUtils_tests.cpp | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/src/OpenColorIO/LookParse.cpp b/src/OpenColorIO/LookParse.cpp index 2b83270e0d..0102d17db1 100644 --- a/src/OpenColorIO/LookParse.cpp +++ b/src/OpenColorIO/LookParse.cpp @@ -84,6 +84,12 @@ const LookParseResult::Options & LookParseResult::parse(const std::string & look tokens.push_back(t); } + if( vec.size() == 0 ) + { + LookParseResult::Token t; + tokens.push_back(t); + } + m_options.push_back(tokens); } diff --git a/tests/cpu/ParseUtils_tests.cpp b/tests/cpu/ParseUtils_tests.cpp index 2af2ce84b0..b46e147d6a 100644 --- a/tests/cpu/ParseUtils_tests.cpp +++ b/tests/cpu/ParseUtils_tests.cpp @@ -388,6 +388,10 @@ OCIO_ADD_TEST(ParseUtils, string_vec_to_int_vec) OCIO_ADD_TEST(ParseUtils, split_string_env_style) { StringUtils::StringVec outputvec; + outputvec = OCIO::SplitStringEnvStyle(""); + OCIO_CHECK_EQUAL(0, outputvec.size()); + outputvec.clear(); + outputvec = OCIO::SplitStringEnvStyle("This:is:a:test"); OCIO_CHECK_EQUAL(4, outputvec.size()); OCIO_CHECK_EQUAL("This", outputvec[0]); @@ -485,6 +489,9 @@ OCIO_ADD_TEST(ParseUtils, join_string_env_style) OCIO_CHECK_EQUAL( "This, is, a, test", OCIO::JoinStringEnvStyle(outputvec) ); outputvec.clear(); + OCIO_CHECK_EQUAL( "", OCIO::JoinStringEnvStyle(outputvec) ); + outputvec.clear(); + outputvec = { "This:is", "a:test" }; OCIO_CHECK_EQUAL( "\"This:is\", \"a:test\"", OCIO::JoinStringEnvStyle(outputvec) ); outputvec.clear(); From dcbd8763afcb574c9d323a7626dc5fc3ef38bb1b Mon Sep 17 00:00:00 2001 From: Jonathan Laroche Date: Thu, 12 Dec 2024 11:03:14 -0500 Subject: [PATCH 4/9] Fix for missing throws, wrong param name, comment on YAML, changed related test --- src/OpenColorIO/Config.cpp | 41 +++++++++++++------ src/OpenColorIO/OCIOYaml.cpp | 3 ++ src/OpenColorIO/ParseUtils.cpp | 5 ++- .../apphelpers/mergeconfigs/OCIOMYaml.cpp | 15 +++---- tests/cpu/ParseUtils_tests.cpp | 21 +++------- 5 files changed, 49 insertions(+), 36 deletions(-) diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index 2b4db90f7a..d270953e6e 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -4005,26 +4005,26 @@ const char * Config::getActiveDisplays() const return getImpl()->m_activeDisplaysStr.c_str(); } -void Config::addActiveDisplay(const char * view) +void Config::addActiveDisplay(const char * display) { - if( !view || !view[0] ) + if( !display || !display[0] ) { throw Exception("Active display could not be added to config, display name has to be a " "non-empty name."); } auto it = std::find(getImpl()->m_activeDisplays.begin(), - getImpl()->m_activeDisplays.end(), view); + getImpl()->m_activeDisplays.end(), display); if( it != getImpl()->m_activeDisplays.end() ) { std::ostringstream os; os << "Active display could not be added to config. An active display named '" - << view << "' already exists."; + << display << "' already exists."; throw Exception(os.str().c_str()); } - getImpl()->m_activeDisplays.push_back(view); + getImpl()->m_activeDisplays.push_back(display); getImpl()->m_displayCache.clear(); @@ -4040,8 +4040,8 @@ void Config::removeActiveDisplay(const char * display) "non-empty name."); } - auto it = std::find(getImpl()->m_activeDisplays.begin(), - getImpl()->m_activeDisplays.end(), display); + auto it = std::find( getImpl()->m_activeDisplays.begin(), + getImpl()->m_activeDisplays.end(), display ); if( it != getImpl()->m_activeDisplays.end() ) { @@ -4056,7 +4056,6 @@ void Config::removeActiveDisplay(const char * display) } getImpl()->m_displayCache.clear(); - AutoMutex lock(getImpl()->m_cacheidMutex); getImpl()->resetCacheIDs(); } @@ -4112,6 +4111,17 @@ void Config::addActiveView(const char * view) "non-empty name."); } + auto it = std::find(getImpl()->m_activeViews.begin(), + getImpl()->m_activeViews.end(), view); + + if( it != getImpl()->m_activeViews.end() ) + { + std::ostringstream os; + os << "Active view could not be added to config. An active view named '" + << view << "' already exists."; + throw Exception(os.str().c_str()); + } + getImpl()->m_activeViews.push_back(view); getImpl()->m_displayCache.clear(); @@ -4123,17 +4133,24 @@ void Config::removeActiveView(const char * view) { if( !view || !view[0] ) { - throw Exception("Active view could not be removed from config, view name has to be a " - "non-empty name."); + throw Exception("Active view could not be removed from config, view name has to be a " + "non-empty name."); } - auto it = std::find(getImpl()->m_activeViews.begin(), - getImpl()->m_activeViews.end(), view); + auto it = std::find( getImpl()->m_activeViews.begin(), + getImpl()->m_activeViews.end(), view ); if(it!=getImpl()->m_activeViews.end()) { getImpl()->m_activeViews.erase(it); } + else + { + std::ostringstream os; + os << "Active view could not be removed from config. An active view named '" + << view << "' could not be found."; + throw Exception(os.str().c_str()); + } getImpl()->m_displayCache.clear(); AutoMutex lock(getImpl()->m_cacheidMutex); diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index 6805b325ba..9678b7b379 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -4999,6 +4999,7 @@ inline void save(YAML::Emitter & out, const Config & config) active_displays.push_back(config.getActiveDisplay(i)); } + // The YAML library will wrap names that use a comma in quotes. out << YAML::Value << YAML::Flow << active_displays; @@ -5010,6 +5011,8 @@ inline void save(YAML::Emitter & out, const Config & config) { active_views.push_back(config.getActiveView(i)); } + + // The YAML library will wrap names that use a comma in quotes. out << YAML::Value << YAML::Flow << active_views; const std::string inactiveCSs = config.getInactiveColorSpaces(); diff --git a/src/OpenColorIO/ParseUtils.cpp b/src/OpenColorIO/ParseUtils.cpp index 64d459a8fe..61875f76a2 100644 --- a/src/OpenColorIO/ParseUtils.cpp +++ b/src/OpenColorIO/ParseUtils.cpp @@ -715,8 +715,9 @@ static int findEndOfName(const std::string & s, size_t start) if(nameEndPos == (int)std::string::npos) { // We reached the end of the list instead - nameEndPos = s.size(); - isEndFound = true; + std::ostringstream os; + os << "The string '" << s << "' is not correctly formatted. It is missing a closing quote."; + throw Exception(os.str().c_str()); } else { diff --git a/src/OpenColorIO/apphelpers/mergeconfigs/OCIOMYaml.cpp b/src/OpenColorIO/apphelpers/mergeconfigs/OCIOMYaml.cpp index 3b65170546..25f6dcd086 100644 --- a/src/OpenColorIO/apphelpers/mergeconfigs/OCIOMYaml.cpp +++ b/src/OpenColorIO/apphelpers/mergeconfigs/OCIOMYaml.cpp @@ -548,14 +548,14 @@ inline void save(YAML::Emitter & out, const ConfigMerger & merger) out << YAML::Key << "active_displays"; StringUtils::StringVec active_displays; - int nDisplays = p->getNumActiveDisplays(); - active_displays.reserve( nDisplays ); - for (int i = 0; i < nDisplays; i++) - { - active_displays.push_back(p->getActiveDisplay(i)); - } - + int nDisplays = p->getNumActiveDisplays(); + active_displays.reserve( nDisplays ); + for (int i = 0; i < nDisplays; i++) + { + active_displays.push_back(p->getActiveDisplay(i)); + } + // The YAML library will wrap names that use a comma in quotes. out << YAML::Value << YAML::Flow << active_displays; out << YAML::Newline; @@ -568,6 +568,7 @@ inline void save(YAML::Emitter & out, const ConfigMerger & merger) active_views.push_back(p->getActiveView(i)); } + // The YAML library will wrap names that use a comma in quotes. out << YAML::Value << YAML::Flow << active_views; out << YAML::Key << "inactive_colorspaces"; diff --git a/tests/cpu/ParseUtils_tests.cpp b/tests/cpu/ParseUtils_tests.cpp index b46e147d6a..dfe53792c6 100644 --- a/tests/cpu/ParseUtils_tests.cpp +++ b/tests/cpu/ParseUtils_tests.cpp @@ -436,10 +436,9 @@ OCIO_ADD_TEST(ParseUtils, split_string_env_style) OCIO_CHECK_EQUAL("a", outputvec[1]); OCIO_CHECK_EQUAL("test", outputvec[2]); - outputvec = OCIO::SplitStringEnvStyle(" This : is \": a: test "); - OCIO_CHECK_EQUAL(2, outputvec.size()); - OCIO_CHECK_EQUAL("This", outputvec[0]); - OCIO_CHECK_EQUAL("is \": a: test", outputvec[1]); + OCIO_CHECK_THROW_WHAT( OCIO::SplitStringEnvStyle(" This : is \": a: test "), + OCIO::Exception, + "The string 'This : is \": a: test' is not correctly formatted. It is missing a closing quote."); outputvec = OCIO::SplitStringEnvStyle(" This : is \": a: test \""); OCIO_CHECK_EQUAL(2, outputvec.size()); @@ -464,22 +463,14 @@ OCIO_ADD_TEST(ParseUtils, split_string_env_style) OCIO_CHECK_EQUAL("a", outputvec[1]); OCIO_CHECK_EQUAL("test", outputvec[2]); - outputvec = OCIO::SplitStringEnvStyle(" This : is \": a: test "); - OCIO_CHECK_EQUAL(2, outputvec.size()); - OCIO_CHECK_EQUAL("This", outputvec[0]); - OCIO_CHECK_EQUAL("is \": a: test", outputvec[1]); - outputvec = OCIO::SplitStringEnvStyle(" This : is \": a: test \""); OCIO_CHECK_EQUAL(2, outputvec.size()); OCIO_CHECK_EQUAL("This", outputvec[0]); OCIO_CHECK_EQUAL("is \": a: test \"" , outputvec[1]); - outputvec = OCIO::SplitStringEnvStyle(" This : is : a: test \""); - OCIO_CHECK_EQUAL(4, outputvec.size()); - OCIO_CHECK_EQUAL("This", outputvec[0]); - OCIO_CHECK_EQUAL("is", outputvec[1]); - OCIO_CHECK_EQUAL("a", outputvec[2]); - OCIO_CHECK_EQUAL("test \"", outputvec[3]); + OCIO_CHECK_THROW_WHAT( OCIO::SplitStringEnvStyle(" This : is : a: test \""), + OCIO::Exception, + "The string 'This : is : a: test \"' is not correctly formatted. It is missing a closing quote."); } OCIO_ADD_TEST(ParseUtils, join_string_env_style) From 8b0a0b94dbace3e551a9e3886147436a7a5d0d66 Mon Sep 17 00:00:00 2001 From: Jonathan Laroche Date: Thu, 12 Dec 2024 12:18:22 -0500 Subject: [PATCH 5/9] Fix for not mixing separators and added tests --- src/OpenColorIO/ParseUtils.cpp | 49 +++++++++++++++++++++++----------- tests/cpu/ParseUtils_tests.cpp | 28 ++++++++++--------- 2 files changed, 48 insertions(+), 29 deletions(-) diff --git a/src/OpenColorIO/ParseUtils.cpp b/src/OpenColorIO/ParseUtils.cpp index 61875f76a2..b25288403c 100644 --- a/src/OpenColorIO/ParseUtils.cpp +++ b/src/OpenColorIO/ParseUtils.cpp @@ -693,15 +693,18 @@ bool StrEqualsCaseIgnore(const std::string & a, const std::string & b) // Find the end of a name from a list contained in a string. // The element of the list are separeted by ":" or ",". // The name can be surrounded by quotes to enable name including theses symbols. -static int findEndOfName(const std::string & s, size_t start) +static int findEndOfName(const std::string & s, size_t start, char sep) { int currentPos = start; int nameEndPos = currentPos; bool isEndFound = false; + std::string symbols = "\""; + symbols += sep; + while( !isEndFound ) { - nameEndPos = s.find_first_of("\",:", currentPos); + nameEndPos = s.find_first_of(symbols, currentPos); if(nameEndPos == (int)std::string::npos) { // We reached the end of the list @@ -726,8 +729,7 @@ static int findEndOfName(const std::string & s, size_t start) currentPos = nameEndPos + 1; } } - else if( s[nameEndPos] == ',' || - s[nameEndPos] == ':' ) + else if( s[nameEndPos] == sep ) { // We found a symbol separating the elements, we stop here isEndFound = true; @@ -746,22 +748,37 @@ StringUtils::StringVec SplitStringEnvStyle(const std::string & str) } StringUtils::StringVec outputvec; - int currentPos = 0; - while( s.size() > 0 && - currentPos <= (int) s.size() ) + auto foundComma = s.find_first_of(","); + auto foundColon = s.find_first_of(":"); + + if( foundComma != std::string::npos || + foundColon != std::string::npos ) { - int nameEndPos = findEndOfName(s, currentPos); - if(nameEndPos > currentPos) + int currentPos = 0; + while( s.size() > 0 && + currentPos <= (int) s.size() ) { - outputvec.push_back(s.substr(currentPos, nameEndPos - currentPos)); - currentPos = nameEndPos + 1; - } - else - { - outputvec.push_back(""); - currentPos += 1; + int nameEndPos = findEndOfName( s, + currentPos, + foundComma != std::string::npos ? ',' : ':' ); + if(nameEndPos > currentPos) + { + outputvec.push_back(s.substr(currentPos, nameEndPos - currentPos)); + currentPos = nameEndPos + 1; + } + else + { + outputvec.push_back(""); + currentPos += 1; + } } } + else + { + // If there is no colon either, + // we consider the string as a single element + outputvec.push_back(s); + } for ( auto & val : outputvec ) { diff --git a/tests/cpu/ParseUtils_tests.cpp b/tests/cpu/ParseUtils_tests.cpp index dfe53792c6..3b10450023 100644 --- a/tests/cpu/ParseUtils_tests.cpp +++ b/tests/cpu/ParseUtils_tests.cpp @@ -417,11 +417,9 @@ OCIO_ADD_TEST(ParseUtils, split_string_env_style) outputvec.clear(); outputvec = OCIO::SplitStringEnvStyle("This:is , a:test "); - OCIO_CHECK_EQUAL(4, outputvec.size()); - OCIO_CHECK_EQUAL("This", outputvec[0]); - OCIO_CHECK_EQUAL("is", outputvec[1]); - OCIO_CHECK_EQUAL("a", outputvec[2]); - OCIO_CHECK_EQUAL("test", outputvec[3]); + OCIO_CHECK_EQUAL(2, outputvec.size()); + OCIO_CHECK_EQUAL("This:is", outputvec[0]); + OCIO_CHECK_EQUAL("a:test", outputvec[1]); outputvec.clear(); outputvec = OCIO::SplitStringEnvStyle(",,"); @@ -452,16 +450,13 @@ OCIO_ADD_TEST(ParseUtils, split_string_env_style) OCIO_CHECK_EQUAL("test", outputvec[2]); outputvec = OCIO::SplitStringEnvStyle(" \"This , is \": a: test "); - OCIO_CHECK_EQUAL(3, outputvec.size()); - OCIO_CHECK_EQUAL("This , is ", outputvec[0]); - OCIO_CHECK_EQUAL("a", outputvec[1]); - OCIO_CHECK_EQUAL("test", outputvec[2]); + OCIO_CHECK_EQUAL(1, outputvec.size()); + OCIO_CHECK_EQUAL("\"This , is \": a: test", outputvec[0]); outputvec = OCIO::SplitStringEnvStyle(" \"This , is \": a, test "); - OCIO_CHECK_EQUAL(3, outputvec.size()); - OCIO_CHECK_EQUAL("This , is ", outputvec[0]); - OCIO_CHECK_EQUAL("a", outputvec[1]); - OCIO_CHECK_EQUAL("test", outputvec[2]); + OCIO_CHECK_EQUAL(2, outputvec.size()); + OCIO_CHECK_EQUAL("\"This , is \": a", outputvec[0]); + OCIO_CHECK_EQUAL("test", outputvec[1]); outputvec = OCIO::SplitStringEnvStyle(" This : is \": a: test \""); OCIO_CHECK_EQUAL(2, outputvec.size()); @@ -497,6 +492,13 @@ OCIO_ADD_TEST(ParseUtils, join_string_env_style) outputvec = { "This", "is \": a: test" }; OCIO_CHECK_EQUAL( "This, \"is \": a: test\"", OCIO::JoinStringEnvStyle(outputvec) ); + + outputvec = { "This, is, a, string", "this, one, too" }; + OCIO_CHECK_EQUAL( "\"This, is, a, string\", \"this, one, too\"" , OCIO::JoinStringEnvStyle(outputvec) ); + + outputvec = { "This", "is: ", "\"a very good,\"", " fine, helpful, and useful ", "test" }; + OCIO_CHECK_EQUAL( "This, \"is: \", \"a very good,\", \" fine, helpful, and useful \", test", + OCIO::JoinStringEnvStyle(outputvec) ); } OCIO_ADD_TEST(ParseUtils, intersect_string_vecs_case_ignore) From 6df1d606fbfd572dc1746c5c575bf6261459cd38 Mon Sep 17 00:00:00 2001 From: Jonathan Laroche Date: Thu, 12 Dec 2024 23:52:51 -0500 Subject: [PATCH 6/9] Fix for LookParser, getNum returning 1 on empty string and added config tests --- src/OpenColorIO/Config.cpp | 14 ++++++++++++++ src/OpenColorIO/LookParse.cpp | 6 ------ src/OpenColorIO/OCIOYaml.cpp | 1 - src/OpenColorIO/ParseUtils.cpp | 2 +- tests/cpu/ParseUtils_tests.cpp | 2 +- tests/cpu/ViewingRules_tests.cpp | 1 + 6 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/OpenColorIO/Config.cpp b/src/OpenColorIO/Config.cpp index d270953e6e..b01532e508 100644 --- a/src/OpenColorIO/Config.cpp +++ b/src/OpenColorIO/Config.cpp @@ -4083,6 +4083,13 @@ const char * Config::getActiveDisplay( int index ) const int Config::getNumActiveDisplays() const { + const int numActiveDisplays = static_cast(getImpl()->m_activeDisplays.size()); + if( numActiveDisplays == 1 && + getImpl()->m_activeDisplays[0].empty() ) + { + return 0; + } + return static_cast(getImpl()->m_activeDisplays.size()); } @@ -4179,6 +4186,13 @@ const char * Config::getActiveView( int index ) const int Config::getNumActiveViews() const { + const int numActiveViews = static_cast(getImpl()->m_activeViews.size()); + if( numActiveViews == 1 && + getImpl()->m_activeViews[0].empty() ) + { + return 0; + } + return static_cast(getImpl()->m_activeViews.size()); } diff --git a/src/OpenColorIO/LookParse.cpp b/src/OpenColorIO/LookParse.cpp index 0102d17db1..2b83270e0d 100644 --- a/src/OpenColorIO/LookParse.cpp +++ b/src/OpenColorIO/LookParse.cpp @@ -84,12 +84,6 @@ const LookParseResult::Options & LookParseResult::parse(const std::string & look tokens.push_back(t); } - if( vec.size() == 0 ) - { - LookParseResult::Token t; - tokens.push_back(t); - } - m_options.push_back(tokens); } diff --git a/src/OpenColorIO/OCIOYaml.cpp b/src/OpenColorIO/OCIOYaml.cpp index 9678b7b379..d29cfc5b2a 100644 --- a/src/OpenColorIO/OCIOYaml.cpp +++ b/src/OpenColorIO/OCIOYaml.cpp @@ -5002,7 +5002,6 @@ inline void save(YAML::Emitter & out, const Config & config) // The YAML library will wrap names that use a comma in quotes. out << YAML::Value << YAML::Flow << active_displays; - out << YAML::Key << "active_views"; StringUtils::StringVec active_views; int nViews = config.getNumActiveViews(); diff --git a/src/OpenColorIO/ParseUtils.cpp b/src/OpenColorIO/ParseUtils.cpp index b25288403c..dbff0f9e5f 100644 --- a/src/OpenColorIO/ParseUtils.cpp +++ b/src/OpenColorIO/ParseUtils.cpp @@ -744,7 +744,7 @@ StringUtils::StringVec SplitStringEnvStyle(const std::string & str) const std::string s = StringUtils::Trim(str); if( s.size() == 0 ) { - return {}; + return { "" }; } StringUtils::StringVec outputvec; diff --git a/tests/cpu/ParseUtils_tests.cpp b/tests/cpu/ParseUtils_tests.cpp index 3b10450023..aefe1d38fb 100644 --- a/tests/cpu/ParseUtils_tests.cpp +++ b/tests/cpu/ParseUtils_tests.cpp @@ -389,7 +389,7 @@ OCIO_ADD_TEST(ParseUtils, split_string_env_style) { StringUtils::StringVec outputvec; outputvec = OCIO::SplitStringEnvStyle(""); - OCIO_CHECK_EQUAL(0, outputvec.size()); + OCIO_CHECK_EQUAL(1, outputvec.size()); outputvec.clear(); outputvec = OCIO::SplitStringEnvStyle("This:is:a:test"); diff --git a/tests/cpu/ViewingRules_tests.cpp b/tests/cpu/ViewingRules_tests.cpp index a58ffaf63f..62e94f2fd9 100644 --- a/tests/cpu/ViewingRules_tests.cpp +++ b/tests/cpu/ViewingRules_tests.cpp @@ -488,6 +488,7 @@ active_views: [] std::stringstream os; os << *config.get(); + OCIO_CHECK_EQUAL(0, config->getNumActiveDisplays()); OCIO_CHECK_EQUAL(os.str(), SIMPLE_CONFIG); // Copy to set active views. From b168c319c39bb8acb1b4970532589b2d8d590c52 Mon Sep 17 00:00:00 2001 From: Jonathan Laroche Date: Fri, 13 Dec 2024 00:08:34 -0500 Subject: [PATCH 7/9] Add test for not necessary quotes --- tests/cpu/ParseUtils_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cpu/ParseUtils_tests.cpp b/tests/cpu/ParseUtils_tests.cpp index aefe1d38fb..f4aaee7d3e 100644 --- a/tests/cpu/ParseUtils_tests.cpp +++ b/tests/cpu/ParseUtils_tests.cpp @@ -400,7 +400,7 @@ OCIO_ADD_TEST(ParseUtils, split_string_env_style) OCIO_CHECK_EQUAL("test", outputvec[3]); outputvec.clear(); - outputvec = OCIO::SplitStringEnvStyle(" This : is : a: test "); + outputvec = OCIO::SplitStringEnvStyle(" \"This\" : is : a: test "); OCIO_CHECK_EQUAL(4, outputvec.size()); OCIO_CHECK_EQUAL("This", outputvec[0]); OCIO_CHECK_EQUAL("is", outputvec[1]); @@ -493,7 +493,7 @@ OCIO_ADD_TEST(ParseUtils, join_string_env_style) outputvec = { "This", "is \": a: test" }; OCIO_CHECK_EQUAL( "This, \"is \": a: test\"", OCIO::JoinStringEnvStyle(outputvec) ); - outputvec = { "This, is, a, string", "this, one, too" }; + outputvec = { "\"This, is, a, string\"", "this, one, too" }; OCIO_CHECK_EQUAL( "\"This, is, a, string\", \"this, one, too\"" , OCIO::JoinStringEnvStyle(outputvec) ); outputvec = { "This", "is: ", "\"a very good,\"", " fine, helpful, and useful ", "test" }; From 51d70eae74442811fc3137332740f6287aefc969 Mon Sep 17 00:00:00 2001 From: Jonathan Laroche Date: Fri, 13 Dec 2024 00:28:03 -0500 Subject: [PATCH 8/9] Fix for comments --- src/OpenColorIO/ParseUtils.cpp | 2 +- tests/cpu/ParseUtils_tests.cpp | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/OpenColorIO/ParseUtils.cpp b/src/OpenColorIO/ParseUtils.cpp index dbff0f9e5f..5e5708c190 100644 --- a/src/OpenColorIO/ParseUtils.cpp +++ b/src/OpenColorIO/ParseUtils.cpp @@ -691,7 +691,7 @@ bool StrEqualsCaseIgnore(const std::string & a, const std::string & b) } // Find the end of a name from a list contained in a string. -// The element of the list are separeted by ":" or ",". +// The element of the list are separeted by a character defined by the sep parameter. // The name can be surrounded by quotes to enable name including theses symbols. static int findEndOfName(const std::string & s, size_t start, char sep) { diff --git a/tests/cpu/ParseUtils_tests.cpp b/tests/cpu/ParseUtils_tests.cpp index f4aaee7d3e..83a377c243 100644 --- a/tests/cpu/ParseUtils_tests.cpp +++ b/tests/cpu/ParseUtils_tests.cpp @@ -449,6 +449,9 @@ OCIO_ADD_TEST(ParseUtils, split_string_env_style) OCIO_CHECK_EQUAL("a", outputvec[1]); OCIO_CHECK_EQUAL("test", outputvec[2]); + // If the string contains a comma, + // it is chosen as the separator character rather than the colon + // (even if it is within quotes and therefore not used as such). outputvec = OCIO::SplitStringEnvStyle(" \"This , is \": a: test "); OCIO_CHECK_EQUAL(1, outputvec.size()); OCIO_CHECK_EQUAL("\"This , is \": a: test", outputvec[0]); From 8e3780928a446b299791a30bf489216f169d2ccf Mon Sep 17 00:00:00 2001 From: Jonathan Laroche Date: Fri, 13 Dec 2024 09:20:10 -0500 Subject: [PATCH 9/9] Fix windows CI --- src/OpenColorIO/ParseUtils.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/OpenColorIO/ParseUtils.cpp b/src/OpenColorIO/ParseUtils.cpp index 5e5708c190..869c4f1d8c 100644 --- a/src/OpenColorIO/ParseUtils.cpp +++ b/src/OpenColorIO/ParseUtils.cpp @@ -695,7 +695,7 @@ bool StrEqualsCaseIgnore(const std::string & a, const std::string & b) // The name can be surrounded by quotes to enable name including theses symbols. static int findEndOfName(const std::string & s, size_t start, char sep) { - int currentPos = start; + int currentPos = static_cast(start); int nameEndPos = currentPos; bool isEndFound = false; @@ -704,17 +704,17 @@ static int findEndOfName(const std::string & s, size_t start, char sep) while( !isEndFound ) { - nameEndPos = s.find_first_of(symbols, currentPos); - if(nameEndPos == (int)std::string::npos) + nameEndPos = static_cast( s.find_first_of( symbols, currentPos ) ); + if( nameEndPos == static_cast(std::string::npos) ) { // We reached the end of the list - nameEndPos = s.size(); + nameEndPos = static_cast( s.size() ); isEndFound = true; } else if( s[nameEndPos] == '"' ) { // We found a quote, we need to find the next one - nameEndPos = s.find_first_of("\"", nameEndPos + 1); + nameEndPos = static_cast( s.find_first_of("\"", nameEndPos + 1) ); if(nameEndPos == (int)std::string::npos) { // We reached the end of the list instead