Adding support for commas in active views list #26
Conversation
larochj
force-pushed
the
larochj/set-active-views-commas
branch
2 times, most recently
from
a3dd78e to
d53c452
Compare
-Adding test cases and some parsing fixes -Fix for join -Fix for bug in split string and move order of functions
larochj
force-pushed
the
larochj/set-active-views-commas
branch
from
d53c452 to
6c41780
Compare
added a test case for SplitStringEnvStyle and JoinStringEnvStyle.
| outputvec.clear(); | ||
|
|
||
| outputvec = { "This", "is \": a: test" }; | ||
| OCIO_CHECK_EQUAL( "This, \"is \": a: test\"", OCIO::JoinStringEnvStyle(outputvec) ); |
There was a problem hiding this comment.
Please add more tests that use commas and quotes, such as:
outputvec = { "This", "is: ", ""a very good,"", " fine, helpful, and useful ", "test" };
There was a problem hiding this comment.
Added this test case and another one
There was a problem hiding this comment.
Actually, please make the first word ""This"", to test that quotes are omitted when they aren't necessary.
There was a problem hiding this comment.
Join would not remove the quotes in this case, but it would not add another around a string that is already surrounded by quotes. I would expect split to remove the quotes (not join).
I changed two test case to touch what I just described.
larochj
force-pushed
the
larochj/set-active-views-commas
branch
from
07e0c57 to
dcbd876
Compare
| outputvec.clear(); | ||
|
|
||
| outputvec = { "This", "is \": a: test" }; | ||
| OCIO_CHECK_EQUAL( "This, \"is \": a: test\"", OCIO::JoinStringEnvStyle(outputvec) ); |
There was a problem hiding this comment.
Actually, please make the first word ""This"", to test that quotes are omitted when they aren't necessary.
larochj
force-pushed
the
larochj/set-active-views-commas
branch
from
5b0fcc7 to
6df1d60
Compare
There was a problem hiding this comment.
If you're able to fix the Windows CI failure, that would be great. Otherwise, go ahead and merge and I will fix it before submitting to OCIO.
Don't worry about the other CI failures. I need to pull over the latest Actions workflow from OCIO main to get that working again on our branch.
larochj
force-pushed
the
larochj/set-active-views-commas
branch
from
8f9854d to
659b7b3
Compare
larochj
force-pushed
the
larochj/set-active-views-commas
branch
from
659b7b3 to
8e37809
Compare
Added a set/get/clear/remove for the active views and displays.
Made changes to the YAML save to avoid repeating the parsing of the string list.