Huge `JSON::ParseException` about crystal HOT 5 OPEN

I suppose we could change the error message to print only the first n characters of the string?

from crystal.

I suppose we could change the error message to print only the first n characters of the string?

Yes. I think I would change it to something like this:

      max_length = 50
      raise JSON::ParseException.new(String.build{ |ret|
        ret << "Couldn't parse #{self} from "
        ret << string[0, max_length].inspect
        if string.size > max_length
          ret << "... (truncated, string had "
          ret << string.bytesize.format('.', '_')
          ret << " B)"
        end
      }, *location)

I'm not sure about adding .inspect though. In a way it's nicer, as you get quotes around the string, so it doesn't look weird if it's truncated. But then again the JSON string itself is harder to read.

And I'm also not sure about a reasonable max_length. Around 50 or 100 I'd say.

I did a quick test to check if any control characters in the data could mess up my terminal if I don't use .inspect. The answer is no, as the lexer would complain first.

But: IMHO .inspect should be added in JSON::Lexer, for that very reason.

-      raise "Unknown escape char: #{char}"
+      raise "Unknown escape char: #{char.inspect}"

-    raise "Unexpected char '#{char}'"
+    raise "Unexpected char #{char.inspect}"

from crystal.

Yeah, that sounds good. Let's go with some value in the range of 50-100 characters and see how it works.

I don't think the total bytesize very valuable. It's just an intermediary raw string, so I don't see much relevance for it.

.inspect makes a lot of sense to ensure proper formatting. I suppose it would be nice if we didn't need to escape quotes, but there's no trivial solution for this.

Maybe this implementation could even be upstreamed into String because I think it might be quite generally useful. String#truncate maybe? This should be a separate change though.

from crystal.

I wonder if it is better to use the string fragment at the error location here, not just the beginning of the string

from crystal.

Hm, good point. It's a bit more complex though because locations from the parser are line/column based and we cannot directly index into the string.

Also, I'm considering there are good options to show the start of string because it gives a clear anchor point. If you start somewhere in the middle, you don't know about context, so it could be confusing.
Perhaps we could also do a combination? Print the start of the string, then omit excess until close to the error location.

from crystal.