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

Add lexer/parser changes for inline table support #71

Merged
merged 3 commits into from
Mar 7, 2017

Conversation

ttacon
Copy link
Contributor

@ttacon ttacon commented Feb 9, 2015

These are the changes I've made so far in reference to issue #70 .

@ttacon ttacon mentioned this pull request Feb 9, 2015
@ttacon ttacon changed the title Add lexer changes for inline table support Add lexer/parser changes for inline table support Feb 10, 2015
@dln
Copy link

dln commented Oct 23, 2015

👍 this would be very nice to see merged.

@@ -424,6 +424,44 @@ name = "Rice"

}

func TestDecodeInlineTable(t *testing.T) {
inlineToml := `
Copy link
Collaborator

Choose a reason for hiding this comment

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

inlineTOML, but just input would be a fine name.

@0xmichalis
Copy link

This would be great to have. @BurntSushi got any time to review this?

@aarondl
Copy link

aarondl commented Oct 12, 2016

@BurntSushi Is there something holding this up? 0.4.0 has been out forever, can we support it yet? :(

@zhaytee
Copy link

zhaytee commented Mar 3, 2017

I needed to use this PR right away (thanks for your work, @ttacon) so I made a fork which merges it into the current upstream (BurntSushi) master. Feel free to use it if you also need a stopgap solution until it makes it into upstream! https://github.com/zhaytee/toml

@0xmichalis
Copy link

We also forked and applied this fix on top. Works like a charm. Gotta love open source.

@aarondl
Copy link

aarondl commented Mar 3, 2017

This is one of my favorite Go repos, but it looks like @BurntSushi has transitioned fully to Rust given Github activity. @cespare seems active, but not here. Let's be honest about this pull request: Opened in 2015, not merged yet... Cespare gave it a brief look... 7 months ago. And it's not just any old pull request, this is part of the roadmap for 0.4.0 support.

Maybe it's time to start the conversation about adding collaborators to this repo or permanent forking if neither have time for it? Hopefully they'll say something in response to something as dire as the word "fork" :)

@BurntSushi
Copy link
Owner

BurntSushi commented Mar 3, 2017

@aarondl I haven't "transitioned fully to Rust." I still use Go every single day. My public Github activity is not a complete picture of my activity. I just haven't had the time to get back to this project. (Some of my Rust projects have old PRs too, although none quite this bad!)

I defer to @cespare on this, but I do think we should get inline table support to be spec conforming. (I'm not necessarily opposed to a fork, but I would like to hear from @cespare. In particular, I would much prefer a transfer of ownership/maintenance over a fork.)

@aarondl
Copy link

aarondl commented Mar 3, 2017

Sounds fine to me. The end goal is simply to have this/that be a first class up-to-spec TOML implementation as it used to be. Sorry for the Rust comment, I did hint that I was trying to be inflammatory to evoke a response though (and it worked!). Many thanks for having created this in the first place and your work on it so far.

@cespare
Copy link
Collaborator

cespare commented Mar 3, 2017

Oh, I'd been waiting for my review comments to be addressed.

I'll re-review, merge, and clean this up shortly.

@cespare
Copy link
Collaborator

cespare commented Mar 4, 2017

I looked at this for a few minutes and I notice that, for one, this allows newlines within inline tables which is specifically disallowed by the spec.

I've set aside some time for working on open-source projects while I'm on vacation next week, so I'll close out this issue then.

@cespare cespare merged commit ff931fa into BurntSushi:master Mar 7, 2017
@cespare
Copy link
Collaborator

cespare commented Mar 7, 2017

I merged this, reworked the tests, and changed to to disallow newlines within inline tables.

Thanks @ttacon for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants