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

Support single quoted keys (#61) #193

Closed
wants to merge 1 commit into
base: master
from

Conversation

3 participants
@kzys
Contributor

kzys commented Oct 19, 2017

This change supports single quoted keys as like quoted keys. However both don't support escape sequences correctly.

According to the spec;

Quoted keys follow the exact same rules as either basic strings or literal strings and allow you to use a much broader set of key names.

Should we fix both of them in this PR?

@moorereason

This comment has been minimized.

Show comment
Hide comment
@moorereason

moorereason Oct 19, 2017

Contributor

If this PR doesn't implement single-quoted keys correctly, then yes, please fix that in this PR.

Contributor

moorereason commented Oct 19, 2017

If this PR doesn't implement single-quoted keys correctly, then yes, please fix that in this PR.

@kzys

This comment has been minimized.

Show comment
Hide comment
@kzys

kzys Oct 19, 2017

Contributor

I've updated the PR to support escape sequence correctly. The logic is pretty similar to lexString(), which should be merged into one function though.

Contributor

kzys commented Oct 19, 2017

I've updated the PR to support escape sequence correctly. The logic is pretty similar to lexString(), which should be merged into one function though.

@moorereason

This comment has been minimized.

Show comment
Hide comment
@moorereason

moorereason Oct 19, 2017

Contributor

@kzys, use gofmt.

Contributor

moorereason commented Oct 19, 2017

@kzys, use gofmt.

@kzys

This comment has been minimized.

Show comment
Hide comment
@kzys

kzys Oct 20, 2017

Contributor

Thanks. Updated the pull request.

Contributor

kzys commented Oct 20, 2017

Thanks. Updated the pull request.

pelletier added a commit that referenced this pull request Oct 21, 2017

@pelletier

This comment has been minimized.

Show comment
Hide comment
@pelletier

pelletier Oct 21, 2017

Owner

Merged a410399. Thank you for implementing that!

Owner

pelletier commented Oct 21, 2017

Merged a410399. Thank you for implementing that!

@pelletier pelletier closed this Oct 21, 2017

@kzys kzys deleted the kzys:quoted-keys branch Oct 24, 2017

pelletier added a commit that referenced this pull request Jan 18, 2018

Fix parsing of single quoted keys (#201)
Patch #193 doesn't work correctly because that must be handled by the
lexer, and `parseKey()` must not handle escape sequences.

Ref #61

pelletier added a commit that referenced this pull request Jan 18, 2018

Fix false positive when running test.sh
Patch #193 introduced a regression in the toml-tests examples, but it was
never caught because test.sh was exiting with a zero status code, even
though the tests failed. This is because of the `|tee` operation when
invoking toml-test, without setting the pipefail option, reporting the
status code of `tee` instead of `toml-test`.

pelletier added a commit that referenced this pull request Jan 18, 2018

Fix false positive when running test.sh (#212)
Patch #193 introduced a regression in the toml-tests examples, but it was
never caught because test.sh was exiting with a zero status code, even
though the tests failed. This is because of the `|tee` operation when
invoking toml-test, without setting the pipefail option, reporting the
status code of `tee` instead of `toml-test`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment