ExJson / xjs-data

A set of tools for manipulating plain JSON-like data
GNU Lesser General Public License v2.1
1 stars 1 forks source link

Add support for (de)serializing infinite numbers #2

Open Treetrain1 opened 3 months ago

Treetrain1 commented 3 months ago

I am attempting to add support for serializing and deserializing infinite numbers (Double.POSITIVE_INFINITY and Double.NEGATIVE_INFINITY)

The output should be Infinity and -Infinity

Implementation may be a bit weird because it still treats Infinity as a WORD token type. Also, since it is hardcoded into the Djs parser, other parsers don't have support. If I can make the token stream treat infinite numbers as a number token then everything would be fine.

Anyway, what do you think?

PersonTheCat commented 3 months ago

I like this idea. Since the specifications for DJS are not finalized, it is definitely possible for us to add additional token types at this point. However, DJS is designed as a subset of XJS. Thus, any tokens in DJS must also be legal in XJS. If I were to support infinity, it would most likely be a lowercase infinity.

In regards to supporting infinity as a NumberToken, I'm not sure I agree. On one hand, it is the responsibility of the tokenizer to decide which type of token each unit of text represents. On the other hand, infinity is a keyword, just like true, false, and null. I am in favor of the current approach.

I am not positive yet whether it is ideal to include an infinity token in DJS. This will need more discussion. Pinging @dshadowwolf. I may merge these changes in the future (before 1.0), but will need to put some thought into it. In the meantime, I think it would be appropriate to support this feature as a DSL extension. We would need some kind of DslTokenProcessor or other system to provide support for additional, domain-specific tokens.

Here's my initial thought on how to implement that:

Perhaps this solution is not ideal. I am open to suggestions. Let's also discuss the implications of adding new tokens into the specifications. If we're changing this now, we need an official BNF or other spec to start with.