On 12/5/19 12:30 AM, Jeff Smith wrote:
On Wed, Dec 4, 2019 at 2:04 PM Nikolay Sivov nsivov@codeweavers.com wrote:
I don't think it makes sense to test for null explicitly. Any invalid char for given context would be a syntax error.
Hi Nikolay,
I agree that it should work that way. However, the tests show that it does not. My belief is that the stream is being treated as a string, so when a null character encountered, it is treating as marking the end of the stream, even though it may actually be a part of the stream. My patch makes sure that condition is being covered, at least for the cases that are being tested.
Putting explicit null checks here and there just to fix some tests is not worth it in my opinion. Instead it should be a part of character range checks, as we do now. E.g. while on whitespace, hitting 0 char would mean you're done with whitespace node, and whatever node is allowed or expected next should handle it.
Thanks, Jeff