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

Empty tag is same as no tag. #233

Closed
wants to merge 1 commit into from
Closed

Conversation

tabatkins
Copy link
Contributor

Don't merge until data model updates (#228) are approved and merged.

@djmattyg007
Copy link
Contributor

Is this to be considered part of the 2.0 spec? My understanding of the 1.0 spec is that there is absolutely a difference between an empty tag and no tag.

@zkat
Copy link
Member

zkat commented Oct 9, 2021

I'm not really into this. I don't want us to start delving into falsiness? I probably missed the conversation about this or forgot about it tho

@tabatkins
Copy link
Contributor Author

Is this to be considered part of the 2.0 spec? My understanding of the 1.0 spec is that there is absolutely a difference between an empty tag and no tag.

While the tests implied there was a difference (and so my impl currently recognizes one) the spec didn't make a statement one way or the other, just as it didn't about whether an empty child list is distinct from no child list (which we recently discussed in #218 and fixed in the testsuite in #219).

I'm not really into this. I don't want us to start delving into falsiness? I probably missed the conversation about this or forgot about it tho

This is based on the data model proposal from #228. In #225 you seemed to prefer empty tag and no tag being treated the same ("both questions above answered with no").

I don't have much of an opinion either way on this, I just want to make sure the tests reflect what we decide on for the data model.

@nichtich
Copy link

nichtich commented Oct 10, 2021

My understanding of the 1.0 spec

Specs try to minimize the number of implicit assumptions but cannot fully eliminate them. Same for other kinds of human communication :-)

I think this issue is misleading because it does not specify the level of description:

  • KDL syntax and parsing rules (as specified in SPEC.md).
    Empty tag and no tag are different on syntax level but the spec does not make clear whether this distinction is relevant or not.

  • KDL test cases
    These are not part of the versioned specification, are they? The test cases seem to differentiate both but the translation rules do not mention tags at all.

  • KDL data model for KQL and KDL Schema
    This does only exist so far and must not be confused with parsing rules, see Start draft of KDL Data Model #228. It's open for discussion. KQL and KDL Schema seem to assume empty tag equal to no tag because there is no way to test for a node without tag (?)

I think this issue refers to the test cases only. As far as I understand these, the purpose of test cases is only to show that a parser can pretty-print KDL documents. It does not require to parse KDL into a data model instance and to serialize it back in normalized form. Nevertheless the current state is confusing.

How about removing test with tag ("") so implementations can choose whether to keep this information?

@zkat
Copy link
Member

zkat commented Oct 12, 2021

ok I thought about it and I think I've settled on "no, I don't want this", so this gets a 👎🏻 from me. I don't want us to null-pun anywhere if we can avoid it.

@tabatkins tabatkins closed this Oct 12, 2021
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.

4 participants