Skip to content

Commit

Permalink
Fix for missing throws, wrong param name, comment on YAML, changed re…
Browse files Browse the repository at this point in the history 
…lated test
  • Loading branch information
@larochj
larochj committed Dec 12, 2024
1 parent d2ebf42 commit dcbd876
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 36 deletions.
41 changes: 29 additions & 12 deletions src/OpenColorIO/Config.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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() )
{
Expand All @@ -4056,7 +4056,6 @@ void Config::removeActiveDisplay(const char * display)
}

getImpl()->m_displayCache.clear();

AutoMutex lock(getImpl()->m_cacheidMutex);
getImpl()->resetCacheIDs();
}
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions src/OpenColorIO/OCIOYaml.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;


Expand All @@ -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();
Expand Down
5 changes: 3 additions & 2 deletions src/OpenColorIO/ParseUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
15 changes: 8 additions & 7 deletions src/OpenColorIO/apphelpers/mergeconfigs/OCIOMYaml.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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";
Expand Down
21 changes: 6 additions & 15 deletions tests/cpu/ParseUtils_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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)
Expand Down

0 comments on commit dcbd876

Please sign in to comment.