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

Clarify key name syntax. #283

Merged
merged 1 commit into from
Jan 15, 2015
Merged

Clarify key name syntax. #283

merged 1 commit into from
Jan 15, 2015

Conversation

mojombo
Copy link
Member

@mojombo mojombo commented Jan 7, 2015

After reflecting on the matter of key names for quite a while, and taking into consideration the many opinions in #65, #67, #220, etc, I've changed my mind on how I think they should be specified. This PR proposes two types of keys: bare and quoted. Bare keys are extremely restrictive [A-Za-z0-9-_]+ while quoted strings can contain almost anything. I like this approach because it is easy to understand (and remember) and guides users to choose simple key names while still making complex names possible. This proposal should also make the parser's job very simple and eliminate any weirdness that could come from having to deal with undelimited Unicode.

cc @wycats, @BurntSushi, @ChristianSi

@@ -319,28 +319,46 @@ Keys are on the left of the equals sign and values are on the right. Whitespace
is ignored around key names and values. The key, equals sign, and value must
be on the same line (though some values can be broken over multiple lines).

Key names may only consist of non-whitespace, non-newline characters excluding
`=`, `#`, `.`, `[`, and `]`.
Keys may be either bare or quoted. **Bare keys** may only contain letters,
Copy link
Contributor

Choose a reason for hiding this comment

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

"letters" should be "ASCII letters" ('Ä' is a letter too).

@BurntSushi
Copy link
Member

I very much like this!

@ChristianSi
Copy link
Contributor

I made a little line note, otherwise this looks good 👍

@redhotvengeance
Copy link
Contributor

👍

@BurntSushi
Copy link
Member

@wycats @alexcrichton do you have any thoughts on this? Will it cause any major churn in Cargo.toml? Unquoted keys allow for - and _, which I know a few packages use, so that's good I think.

(For others: Cargo is Rust's package manager, and it uses TOML as its configuration format. If they aren't already the biggest users of TOML, I'm assuming they will be soon. So I think it's good to check with them before we make backwards incompatible changes.)

@mojombo mojombo mentioned this pull request Jan 7, 2015
@mk-pmb
Copy link

mk-pmb commented Jan 7, 2015

👍 although a lot of my .ini files won't be valid TOML anymore.

@alexcrichton
Copy link
Contributor

Bare keys being so restrictive was somewhat surprising to me, but I think we're fine on Cargo's end. I believe we currently have the exact same restrictions on package names (which can be used as a bare key), and all other keys are basically the same.

This sounds like a great idea, and thanks for the heads up!

@flaviut
Copy link

flaviut commented Jan 7, 2015

👍, although in the interest of compatibility with existing ini files, I'd also like clarification on how #220 (comment) should work.

@mojombo
Copy link
Member Author

mojombo commented Jan 7, 2015

@flaviut As much as I'd love for TOML to be somehow compatible with INI files, I don't really see it happening. Since inception, TOML has diverged from INI (if that's even possible, given the non-standardization of INI), and we must make choices that are right for TOML even if that means giving up on the dream of TOML-as-superset-of-INI.

@mojombo
Copy link
Member Author

mojombo commented Jan 8, 2015

I believe we currently have the exact same restrictions on package names (which can be used as a bare key), and all other keys are basically the same.

@alexcrichton Sweet. I'd love to verify that, just to make sure TOML will continue to be easy to use for Cargo. Think you can take a look and see, or point me to the proper place in the Cargo source? Thanks!

@alexcrichton
Copy link
Contributor

Think you can take a look and see, or point me to the proper place in the Cargo source?

Sure, this is actually validated in our registry, crates.io, when crates are published.

@mojombo
Copy link
Member Author

mojombo commented Jan 8, 2015

@alexcrichton Excellent, thank you for checking! Do you know if @wycats is around? I'd love to have him weigh in before I merge this.

@wycats
Copy link
Contributor

wycats commented Jan 8, 2015

@mojombo I think the biggest issue here is that ünicode-crate = "1.0" (which we don't support now, but could) would require quotes. I don't think the quotes are such a big deal, but would they be expected?

What do you think?

@mojombo
Copy link
Member Author

mojombo commented Jan 13, 2015

@wycats Yes, with this PR you'd need to specify "ünicode-crate" = "1.0". Idealistically I'd love to support unicode in key names, but I think the complexity and ambiguity is just too much to make it worthwhile. Quoting lets a reader know that something unexpected might be going on in the key and force them to look twice, reducing unnecessary confusion and mistakes.

mwanji added a commit to mwanji/toml4j that referenced this pull request Jan 14, 2015
@mojombo
Copy link
Member Author

mojombo commented Jan 15, 2015

I'm going to merge this PR, as I strongly believe it's in the right direction. If we need to tweak it, we can do so in another PR. Thanks for the feedback, everyone. One step closer to 1.0!

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.

8 participants