Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XML string array errors feedback #2357

Merged
merged 10 commits into from
Jun 2, 2023

Conversation

MelReyCG
Copy link
Contributor

@MelReyCG MelReyCG commented Mar 27, 2023

This PR aims to improve the error messages from XML string array in a more user-friendly way:

  • It adds more context to the parsing exceptions so the user can retrieve the error cause faster,
  • In order to throw these exception properly, a modification of LvArray::input::stringToArray() is needed (LvArray PR #280),
  • It modifies the generated schema.xsd so these errors can be detected by xml validation,

This PR solves the case 1 of issue #2320.
LvArray PR #280

@MelReyCG MelReyCG requested a review from untereiner March 31, 2023 07:11
string rp = "[^*?<>\\|:\";,\\s]+\\s*";//"[^*?\\&lt;\\&gt;\\|:\\&quot;,\\s]+\\s*";

// Regex to match a path: a string that does not contain any space nor the characters *?<>|:",
string rpe = "[^*?<>\\|:\";,\\s]*\\s*";//"[^*\\?&lt;\\&gt;\\|:\\&quot;,\\s]*\\s*";
Copy link
Contributor

Choose a reason for hiding this comment

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

could be static constexpr auto instead of string ?

Copy link
Contributor Author

@MelReyCG MelReyCG Apr 3, 2023

Choose a reason for hiding this comment

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

I'm afraid we cannot because of r1 and r2s need string operations. We would need string_view here.

@MelReyCG MelReyCG requested a review from cssherman May 15, 2023 08:12
Copy link
Contributor

@TotoGaz TotoGaz left a comment

Choose a reason for hiding this comment

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

Great, thx @MelReyCG
@cssherman Do you think there are any implications with those regex in our global process (python validation or anything else)?

string rs = "[^,\\{\\}]*";
// Regex to match a string that can't be empty and does not contain any whitespaces nor the characters ,{}
// TODO c++17: Move to static constexpr std::string_view
string rs = "[^,\\{\\}\\s]+\\s*";
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to take action with potential initial spaces (like toto) or is it OK?

Copy link
Contributor

@cssherman cssherman May 18, 2023

Choose a reason for hiding this comment

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

This pattern will be permissive of spaces at the end of the string, but not at the beginning, which should be fine, right?

"hello": match
"hello world": no match
"hello, world": no match
" hello": no match
"hello ": match

@MelReyCG MelReyCG added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label May 30, 2023
@wrtobin wrtobin merged commit 186c7ad into develop Jun 2, 2023
@wrtobin wrtobin deleted the bugfix/rey/2320-xml-errors-feedback-3 branch June 2, 2023 17:44
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EPIC] Improve feedback to user on XML errors
6 participants