Fix for missing throws, wrong param name, comment on YAML, changed re…

…lated test
autodesk-forks · Dec 12, 2024 · 07e0c57 · 07e0c57
1 parent d2ebf42
commit 07e0c57
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 34 deletions.
diff --git 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();
@@ -4127,13 +4137,20 @@ void Config::removeActiveView(const char * view)
                             "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
@@ -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
@@ -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
@@ -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
@@ -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)