One way that error messages could be more helpful

[brunchboy]

Opening this by request of @GreyCat. Last night I wasted a lot of time because in an earlier version of this line: https://github.com/Deep-Symmetry/beat-link/blob/5da596c526768a3f6df40d044754845f6397dc67/kaitai/pdb.ksy#L211 I had forgotten that the ksc expression language uses and for conjoining boolean expressions, rather than && like most C-derived languages I generally use, so I had && instead of the and there. Unfortunately, the error message I received was:

End:1:35 ..."&& (num_ro"

That gave me no context whatsoever, not even the line number. It would have been far more helpful to say something like:

Unrecognized token "&&" at line 211, column 44.

That would have at least sent me to the manual to look for the right thing. Since this is probably a common mistake, you could even be extra nice and say something like:

Unrecognized token "&&" at line 211, column 44, did you mean "and"?

Something similar could be offered for || and or.

Even looking at the exception, now that I know that trick, did not help:

Exception in thread "main" io.kaitai.struct.exprlang.Expressions$ParseException: End:1:35 ..."&& (num_ro"
	at io.kaitai.struct.exprlang.Expressions$.realParse(Expressions.scala:178)
	at io.kaitai.struct.exprlang.Expressions$.parse(Expressions.scala:170)
	at io.kaitai.struct.format.InstanceSpec$.$anonfun$fromYaml$1(InstanceSpec.scala:44)
	at scala.Option.map(Option.scala:146)
	at io.kaitai.struct.format.InstanceSpec$.fromYaml(InstanceSpec.scala:44)
	at io.kaitai.struct.format.ClassSpec$.$anonfun$instancesFromYaml$1(ClassSpec.scala:211)
	at scala.collection.TraversableLike.$anonfun$map$1(TraversableLike.scala:234)
	at scala.collection.MapLike$MappedValues.$anonfun$foreach$3(MapLike.scala:253)
	at scala.collection.TraversableLike$WithFilter.$anonfun$foreach$1(TraversableLike.scala:789)
	at scala.collection.immutable.Map$Map3.foreach(Map.scala:176)
	at scala.collection.TraversableLike$WithFilter.foreach(TraversableLike.scala:788)
	at scala.collection.MapLike$MappedValues.foreach(MapLike.scala:253)
	at scala.collection.TraversableLike.map(TraversableLike.scala:234)
	at scala.collection.TraversableLike.map$(TraversableLike.scala:227)
	at scala.collection.AbstractTraversable.map(Traversable.scala:104)
	at io.kaitai.struct.format.ClassSpec$.instancesFromYaml(ClassSpec.scala:207)
	at io.kaitai.struct.format.ClassSpec$.fromYaml(ClassSpec.scala:107)
	at io.kaitai.struct.format.ClassSpec$.$anonfun$typesFromYaml$1(ClassSpec.scala:201)
	at scala.collection.TraversableLike.$anonfun$map$1(TraversableLike.scala:234)
	at scala.collection.immutable.HashMap$HashMap1.foreach(HashMap.scala:231)
	at scala.collection.immutable.HashMap$HashTrieMap.foreach(HashMap.scala:462)
	at scala.collection.TraversableLike.map(TraversableLike.scala:234)
	at scala.collection.TraversableLike.map$(TraversableLike.scala:227)
	at scala.collection.AbstractTraversable.map(Traversable.scala:104)
	at io.kaitai.struct.format.ClassSpec$.typesFromYaml(ClassSpec.scala:199)
	at io.kaitai.struct.format.ClassSpec$.fromYaml(ClassSpec.scala:103)
	at io.kaitai.struct.format.ClassSpec$.fromYaml(ClassSpec.scala:224)
	at io.kaitai.struct.formats.JavaKSYParser$.fileNameToSpec(JavaKSYParser.scala:29)
	at io.kaitai.struct.formats.JavaKSYParser$.localFileToSpecs(JavaKSYParser.scala:18)
	at io.kaitai.struct.JavaMain.compileOneInput(JavaMain.scala:237)
	at io.kaitai.struct.JavaMain.$anonfun$run$1(JavaMain.scala:212)
	at scala.collection.TraversableLike.$anonfun$map$1(TraversableLike.scala:234)
	at scala.collection.immutable.List.foreach(List.scala:389)
	at scala.collection.TraversableLike.map(TraversableLike.scala:234)
	at scala.collection.TraversableLike.map$(TraversableLike.scala:227)
	at scala.collection.immutable.List.map(List.scala:295)
	at io.kaitai.struct.JavaMain.run(JavaMain.scala:210)
	at io.kaitai.struct.JavaMain$.main(JavaMain.scala:201)
	at io.kaitai.struct.JavaMain.main(JavaMain.scala)

In the end, I spent a long time trying to separate the expression into simpler sub-expressions using additional parse instances to try to figure out what was going on, and eventually slapped my forehead and vented in the Gitter chat. Hopefully we can spare future adopters such fun. 😄

[GreyCat]

Thanks for sharing this. Unfortunately, I don't think there's a lot we can do here. Namely, this:

  Unrecognized token "&&" at line 211, column 44.

... is, unfortunately, out of our limits now, at least until we'd get our own YAML parser. Right now, we get just a ready-made tree, and there's no way we can get anything like that (i.e. with line numbers) — best we can do (and that's actually what we strive to do) is getting a YAML path like types/foo/seq/3/repeat-expr to get an idea where the problem happened.

Also, Unrecognized token "&&" is probably also not doable, as our expression parser does not work like that — we can't get a "token" unless we know it.

This idea, on the other hand, might be doable:

Unrecognized token "&&" at line 211, column 44, did you mean "and"?

I mean, if we just analyze whatever is going on right after the position in line where we stopped, and if first 2 symbols of that is &&, then we might suggest did you mean "and"?.

So, to summarize, I guess the best we can do right now in theory is:

/types/page/instances/num_rows/value: Unable to parse expression at line 1, column 35, near "&& (num_ro" -- did you mean "and"?

[brunchboy]

That would be a perfectly fine approach! I’m more used to seeing line/column type coordinates because most parsers make them the easiest to obtain, but a path-based error like that works almost as well.

[brunchboy]

Do you know if anyone has built an Emacs extension that can interpret your path-based error messages so that the “jump to next error” functionality works, at least approximately? If not, I might investigate that… or, maybe not, since there generally seems to be only one error output before ksc gives up, so it would not be that useful anyway.

[GreyCat]

Should be fixed in current compiler. Test that proves it: expr_wrong_and.ksy. Error message would look similar to

/instances/both: parsing expression 'foo == 1 && bar == 2' failed on 1:10, expected "or" | CharsWhile(Set( , n)) | "\\\n" | End, did you mean 'and'?

I totally feel like we can do better with "expected ..." section of the message — but I don't quite understand exactly what is the better alternative.

[GreyCat]

Do you know if anyone has built an Emacs extension that can interpret your path-based error messages so that the “jump to next error” functionality works, at least approximately?

Unlikely, I'd love to do such extension too ;)
Although, yeah, ksc stops processing file on the very first problem encountered, and, instead of parsing text error messages, using ksc --ksc-json-output might be a more attractive option.

[brunchboy]

That improved message helps! And it does look like it will be much harder to do something about the “expected…” section. It seems to at least be trying, but making a concise, human-readable format of the structure it seems to be working with is likely very challenging.

[GreyCat]

Closing the issue for now.