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 where and how dotted keys define tables #848

Closed
wants to merge 0 commits into from

Conversation

eksortso
Copy link
Contributor

@eksortso eksortso commented Sep 6, 2021

Removes and replaces language that could potentially allow dotted keys in a parent table of an existing table to inject key/value pairs into subtables of the existing table. Closes #846.

@marzer
Copy link
Contributor

marzer commented Sep 6, 2021

Maybe an additional counter-example too? Both @hukkin and I would probably have benefited from one during implementations. One good example is always better than (or significantly assists) wording tweakery imo.

(Plus implementers can drop them directly into unit tests which is extremely useful)

@hukkin
Copy link
Contributor

hukkin commented Sep 6, 2021

Are we accidentally banning something like the following snippet?

a.x = 0
a.y = 0

If dotted keys create and define tables unconditionally now, while still (quoting the spec)

Since tables cannot be defined more than once

do we have the problem that line 2 defines table "a" for the second time, rendering the snippet illegal?

@ChristianSi
Copy link
Contributor

ChristianSi commented Sep 6, 2021

Yeah, I think that the "provided that" clause was indeed meant to cover the case mentioned by @hukkin : Two or more dotted keys given in the same [table] block are of course allowed to refer to the same table name without the second being considered an illegal "redefinition" of the table defined by the first one – otherwise dotted keys would be next to useless! So, the "provided that" clause is indeed useful and should stay, but to avoid ambiguities I would propose to extend it as follows:

provided that such tables were not previously created by another dotted key given under the same [table] header.

Strictly speaking that should already be sufficient, but for clarity @eksortso 's new sentence is certainly a good idea, though I would suggest to change the wording slightly:

Any such table must have all its key/value pairs defined under the same [table] header, or in the root table if defined before all headers.

(Emphasis is only used to change which words were added or changed – it's not meant to actually go in the spec.)

Adding some more examples of illegal stuff would indeed be useful too – maybe based on Snippet B from #846.

@pelletier
Copy link
Contributor

Clarifying question on this change: how does this impact inline tables?

For example:

a = { b.c = 1, b.d = 2 }

Is this valid? Or is it considered invalid because b.c defines the table at a.b, and therefore b.d would re-define it, which would be an error?

@ChristianSi
Copy link
Contributor

@pelletier: Your example is valid. All keys within the same block jointly define tables. An inline table is a block, while outside of inline tables a block extends from one explicitly written [table header] to the next.

(Note that the spec doesn't talk about "blocks" – that's just my way of explaining it.)

toml.md Outdated
provided that such tables were not previously created.
Dotted keys create and define a table for each key part before the last one. Any
such table must have all its key/value pairs defined under the current `[table]`
header, or in the root table if defined before all headers.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
header, or in the root table if defined before all headers.
header, or in the root table if defined before all headers, or in one inline table.

@ChristianSi thank you! I think this change would have helped me. At that point I wonder if your concept of block should be defined in the spec, it seems a clearer way to express that behavior.

@ChristianSi
Copy link
Contributor

Why is this not yet merged, by the way?

@eksortso
Copy link
Contributor Author

I accidentally opened a PR on my master branch. I'll follow up with a new PR with the same changes.

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.

Can dotted keys insert into already-defined [tables]?
5 participants