-
Notifications
You must be signed in to change notification settings - Fork 9
592 improvement hdf5 dataset as regex #597
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
base: main
Are you sure you want to change the base?
592 improvement hdf5 dataset as regex #597
Conversation
Each string is the name of a dataset to create in the file on first | ||
access, with the type described in the value. The string key can also be | ||
access, with the type described in the value. The string key is | ||
a regular expression (regex), and be used to define "generic keys", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we add a reference to the regex's standard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Patterns and replacement strings support the following regular expression grammars:
- Modified ECMAScript regular expression grammar. This is the default grammar.
- Basic POSIX regular expression grammar.
- Extended POSIX regular expression grammar.
- The regular expression grammar used by the awk utility in POSIX.
- The regular expression grammar used by the grep utility in POSIX. This is effectively the same as the basic POSIX regular expression grammar, with the addition of newline '\n' as an alternation separator.
- The regular expression grammar used by the grep utility, with the -E option, in POSIX. This is effectively the same as the extended POSIX regular expression grammar, with the addition of newline '\n' as an alternation separator in addition to '|'.
Some grammar variations (such as case-insensitive matching) are also avaliable, see this page for details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If no grammar is chosen, ECMAScript is assumed to be selected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose to add at the end of this paragraph.
For the regex grammar, we used the default grammar in C++: ECMAScript.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd put:
The regex use the Modified ECMAScript regular expression grammar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, with your proposition@jbigot.
I suggest to add explicitly the grammar when we create a regex:
std::regex dset_regex(dset_name_string, std::regex::ECMAScript);
instead of
std::regex dset_regex(dset_name_string);
Just in the case the default grammar changes in the future.
This PR correspond to fix the comment in the previous PR #583
List of things to check before making a PR
Before merging your code, please check the following:
.clang-format
;Fix #issue
keyword to autoclose the issue when merged.