-
Notifications
You must be signed in to change notification settings - Fork 185
Addition of checks and use of stdlib_experimental_ascii #82
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
Conversation
yes I think it's fine/necessary to use |
Thank you @jvdp1 for doing this, it looks really good. I left some comments regarding the API and testing. |
Thanks for the review. |
In your last commit, in the tests I think you forgot to add io in the 3rd test from last. (I am on my phone, so I can't comment there directly.)
…On Sun, Jan 5, 2020, at 9:44 AM, Jeremie Vandenplas wrote:
>
> Thank you @jvdp1 <https://github.com/jvdp1> for doing this, it looks really good. I left some comments regarding the API and testing.
Thanks for the review.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#82?email_source=notifications&email_token=AAAFAWAMZPEYHRA6CLBHG4TQ4IFABA5CNFSM4KC2LVWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEID2WXQ#issuecomment-570927966>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWC5PDMQAP4A7WCAJSDQ4IFABANCNFSM4KC2LVWA>.
|
Resolved! |
Perfect! I think it looks good now. +1 to merge. Go ahead and merge it if you want.
…On Sun, Jan 5, 2020, at 10:35 AM, Jeremie Vandenplas wrote:
>
> In your last commit, in the tests I think you forgot to add io in the 3rd test from last. (I am on my phone, so I can't comment there directly.)
Resolved!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#82?email_source=notifications&email_token=AAAFAWBSWCWM3CRHCDSH4TDQ4IK6DA5CNFSM4KC2LVWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEID3UBA#issuecomment-570931716>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWFV7PSVGON5DXLHNRDQ4IK6DANCNFSM4KC2LVWA>.
|
Additions/changes:
parse_mode
to return errors for modes such asrr
,rw
,r++t
,rbt
. (Note: I couldn't add these tests in CMake, or it should be one per file)open
can return theiostat
if requestedwhitechar
is now replaced byis_blank
@scivision : Because I added checks in the function
parse_mode
, it uses againif
statements instead ofselect case
. Good for you?