-
Notifications
You must be signed in to change notification settings - Fork 761
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Trailing commas #802
Comments
I think that's an issue whatever service is sending you json right? |
maybe we should make lenient readers read this? |
@ZacSweers trailing commas are usually not allowed in Json, but since they are allowed in ECMA5 it would be really helpful if we have a feature of allowing it. something along this Jackson trailing commas support |
ECMA5 is 10 years old, is that still relevant today? |
JavaScript has allowed trailing commas in array literals since the
beginning, and later added them to object literals (ECMAScript 5) and most
recently (ECMAScript 2017) to function parameters. So trailing commas on
browser works fine.
It's an edge case no doubt, but at times you can't dictate the 3rd party
service providers and they expect mobile devices to be able to parse json
which is being passed on browser.
But I get how it shouldn't be prioritized, so happy to wait on it.
…On Fri, 22 Feb 2019, 3:28 am Zac Sweers, ***@***.***> wrote:
ECMA5 is 10 years old, is that still relevant today?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#802 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEcWsipuF4JLen0y5qj7fVuzzwTMQ--wks5vPx2kgaJpZM4acKNG>
.
|
I agree that leniency could handle this if it's easy but it should remain invalid by default. |
Lenient mode handles this. Only problem is we treat the comma as an absent null like |
So lenient mode should handle trailing commas now (in v1.9.3) or by Icebox milestone? I'm trying to parse this JSON: {
"id": 1,
"watermark": {
"x": 772,
"y": 1130,
"width": 221,
"height": 70,
}
} Moshi throws Also, what is best practice workaround for this (considering there is no control over JSON source)? Modifying adapter, or sanitizing JSON before reading (regexp)? |
@killvetrov we’re strict about these currently, even in lenient mode. We should fix this. |
There is an PR since last year, why isn't it being reviewed? This issue is quite annoying |
Thank you for a very helpful library. Is there any way to get past the
trailing comma
issue with moshi?Sample json
{ "name": "Khan", "greeting": "Hello, how are you", }
If you can see there is an extra comma and moshi rightly sends out an exception.
com.squareup.moshi.JsonEncodingException: Expected name at path $.greeting
Looking for any workaround for this issue. TIA
The text was updated successfully, but these errors were encountered: