-
Notifications
You must be signed in to change notification settings - Fork 173
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
Allow for UTF-8 field values in header regular expression #726
base: master
Are you sure you want to change the base?
Conversation
Use `[:print:]` in the header regex and note that for ASCII it is equivalent to `[ -~]` and that the aim is to forbid control characters. Fixes samtools#719.
I'm unsure on the extra brackets also. My inclination though is it's probably not worth the hassle of inventing our own syntax and just going with the official double bracket style. I'm guessing the extra brackets however were to permit things like |
|
Which specific |
Ah, yes, you're right. I had a set operation wrong when I tested with an example. |
I see I was assigned this in the last meeting. Personally my preference is |
Use
[:print:]
in the header regex and note that for ASCII it is equivalent to[ -~]
and that the aim is to forbid control characters. Fixes #719.To be honest, I'm tempted to add the extra
[]
to the\cclass
definition and waste a bit of space each time this appears rather than add the “For brevity” sentence.An alternative to this PR might be to just leave the regex as
[ -~]
and add a footnote explaining that this is an oversimplification for fields that allow Unicode values.