Skip to content

Conversation

@arng40
Copy link
Contributor

@arng40 arng40 commented Nov 4, 2025

To reference an array of groups aka groupNameRefArray, we ensure we have to write attribute="{*}" .
We cannot longer write for example : attribute="*"

Also allow to have spaces spaces before and after a value in an attribute :

someAttributes="{ 0.0, 1.0, 10.0, 100.0 }"
coordinates="   { 0.0, 1.0, 10.0, 100.0 }"

@arng40 arng40 self-assigned this Nov 4, 2025
@arng40 arng40 changed the title Enforce arrayRef regex Enforce arrayRef regex & loosens xml spacing rules Nov 4, 2025
@arng40 arng40 changed the title Enforce arrayRef regex & loosens xml spacing rules refactor: Enforce arrayRef regex & loosens xml spacing rules Nov 4, 2025
@arng40 arng40 changed the title refactor: Enforce arrayRef regex & loosens xml spacing rules refactor: Enforce arrayRef regex & Handling and XML Spacing Rules Nov 5, 2025
@arng40 arng40 added ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Nov 12, 2025
Copy link
Contributor

@MelReyCG MelReyCG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need a few more work on unit tests

Comment on lines 381 to 384
std::string groupNameComp;
std::istringstream ss( input.m_valueToTest );
ss >> groupNameComp >> std::ws;
EXPECT_STREQ( groupNameComp.c_str(), groupName.c_str() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please revert

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your last fix...

      EXPECT_STREQ( groupName.c_str(), groupName.c_str() );

... is like a a == a check, it does not check anything.

Comment on lines 381 to 384
std::string groupNameComp;
std::istringstream ss( input.m_valueToTest );
ss >> groupNameComp >> std::ws;
EXPECT_STREQ( groupNameComp.c_str(), groupName.c_str() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your last fix...

      EXPECT_STREQ( groupName.c_str(), groupName.c_str() );

... is like a a == a check, it does not check anything.

for( GroupNameTest const & input : workingInputs )
{
EXPECT_NO_THROW( xmlWrapper::stringToInputVariable( groupName, input.m_valueToTest, input.m_regex ) );
EXPECT_STREQ( input.m_valueToTest.c_str(), groupName.c_str() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the place that needs to be reworked.

@arng40 arng40 requested a review from MelReyCG January 21, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: ready for review type: cleanup / refactor Non-functional change (NFC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants