Require node terminator before eof

[zkat]

I think we should make it a requirement to have either a newline or a ; as the last character in a document. This means that accidentally truncated files will simply fail to parse.

on the flip side, it means that folks who don't put a final newline in their files still get obnoxious errors, which kinda sucks but I assume most editors will help people do the right thing here?

[tabatkins]

Sounds reasonable to me. It's easy to configure most text editors to always ensure a trailing newline, I think. (At least, Sublime is easy.)

[nitz]

Personal preference: I can't stand files that don't end in a newline. Makes me feel claustrophobic or something.

I like the idea of an explicit terminator, but I don't know how I feel about it being required. In syntaxes like JSON and XML, final terminators are "obvious" because they're part of the the natural flow language. With kdl, I feel like an explicit terminator combats directly with the direct design. Like you said in the readme, kdl "looks like you're invoking a bunch of CLI commands". An explicit terminator seems to be out of place with that. I think it'd be a little 'foot-shotgunny' to require it. What might be a good compromise would perhaps be a "warning" of sorts — the parser would parse everything it can and return it to the user, but with a notice/flag that says "this document was not explicitly terminated". That way, the users who don't care can just continue as is, but those who expect explicit termination would know to reject (or distrust) the results.

I do like the idea of an optional "this is the end of this config", similarly to how yaml uses --- as document starts to allow multiple documents in one physical file or stream. A newline (or maybe any whitespace? non-identifier?) followed by a single ; as an "end of document" seems like it could potentially be very useful. This of course adds the (potential) complexity of "multiple documents" which might go against the stated goals too.

I feel like I've done nothing in my opinion here but further complicate things. 🙃

[lilyball]

YAML’s “this is the end” is intended to allow YAML to be embedded in a larger document. IIRC it had an optional “this is the start” too.

I’m not convinced it’s really all that useful though. Particular use-cases like using YAML as the header of a static site page can always define their own end-of-document tag themselves anyway. I have yet to see a scenario where being able to detect the end of a config file embedded in another file in the general case for arbitrary files is useful.

[nitz]

I've only directly dealt with yaml in a few places, and most of them that have used the multiple documents certainly could have gone other routes but it's a decently elegant way of things like specifying multiple objects without needing to be as an array with a root level element or the sort. Something like the way urlwatch uses each document as a separate job, or how reddit's automoderator uses them as a separate document per rule. Unity3D uses multiple documents so each component it serializes in a scene ends up in it's own document.

Again, sure, most of these things could be solved with lists/arrays, but with the "cli style invocation" of kdl, multiple documents could make sense.

[lilyball]

For KDL I’d prefer to just wrap each doc in a node.

job {
  url "http://example.com/search.cgi"
  data "button=Search&q=something&category=4"
}
job {
  url "http://example.com/foo"
  method "PURGE"
}

[lilyball]

I’m not strongly opposed to this idea, but I’m not sure what we gain from it. Accidentally truncating a document isn’t AFAIK much of an issue. It was more of an issue with HTTP/0.9 responses where truncated documents couldn’t be detected, but that’s not really a concern for KDL.

If we do go ahead with this we should just require a trailing newline rather than using ; for this.

[nitz]

Is there often a case where you're reading a stream of data that you don't know the size of beforehand? I'm pretty unfamiliar with the inner workings of NPM.

My fear with a trailing newline as being a terminator leaves you open to a break in communication after a newline seeming "successfully terminated" in that case. (Like, if a server was generating kdl lines on the fly, sending them in packets of lines at a time, and having the communication breakdown after one packet but before the full document was transferred.)

[lilyball]

Any kind of potentially-truncated communication mechanism should have explicit framing to make truncation obvious. This is not a problem that should be handled on a per-format behavior.

@zkat Why was truncation happening at NPM?

If we do require anything, I think the trailing newline is sufficient. A semicolon is just going to end up being a hazard for anyone writing documents by hand, and will also make it hard to append to KDL files.

[zkat]

Why was truncation happening at NPM?

Mostly misbehaving third-party servers and proxies, though we had a bug once in the main registry where things would get cut off and it was aaawwwful. I'm just saying, this sort of thing happens, and not every tool actually checks Content-Length (if it's not using chunked transfer)

[lilyball]

I still feel like this is something to be solved at the protocol level via framing. KDL isn’t a network protocol, it’s a file format. If you’re going to be sending KDL files across the network and don’t trust HTTP’s built-in framing then wrap it yourself. That wrapping could even just be something like embedding the whole KDL document in a KDL node, e.g. echo 'doc {'; cat doc.kdl; echo '}'.

[nitz]

I agree with that @lilyball. HTTP transfers an html document, and if it gets cut mid element name, it's not up to the html document to specify it's end (despite having the option for closing </html> tags and the sort,) but rather the protocol's job.

[dezren39]

my only thought is that if you require it, then someone will eventually make a variant which does not

[xSke]

I don't think it's reasonable to assume most editors are going to do the right thing in terms of trailing newlines here. I'm sure Sublime Text or VS Code will, but Notepad, TextEdit, nano... might not. Maybe I've misread the target audience here but it would be a very obtuse error for someone who's not too tech-savvy just trying to Make Something Work.

Enforcing a trailing visible token makes sense though, IMO. Whether that's a closing brace, semicolon, or something else.

[jlkiri]

This means that accidentally truncated files will simply fail to parse.

Isn't is true that accidentally truncated files will mostly fail to parse regardless of whether there is a ; or newline in the end? For example an unclosed string is an error anyway. I see how without explicit EOF in some cases it will accidentally succeed to parse (when we don't want it to), for example

node { 
   /* stuff */
}
// no newline

So maybe explicit EOF is a good idea.
But suppose we require EOF (newline) and the document has two nodes and is accidentally truncated at the newline between them.

node1 {
   /* stuff */
}
<- newline here, included in the response or whatever, everything below is not included
node2 {
   /* stuff */
}

Then the document parses OK as if it has a trailing newline but is actually truncated so our EOF requirement didn't help us. I guess that's why ; might be a better idea, because it's not a valid linespace.

[zkat]

@Lucretiel found it! ^

[zkat]

I've converted this into an issue so we can make sure to resolve this by 1.0.0: #116