Add new JsonWriteFeature.ESCAPE_FORWARD_SLASHES
#1197
Conversation
|
I figured we should do #1198 separately. |
| * | ||
| * @since 2.17 | ||
| */ | ||
| protected int[] escapeForwardSlash(int[] escapes) { |
There was a problem hiding this comment.
Should not be added here since this is not JSON-specific type despite name. So rather add in JsonGeneratorImpl, shared base class for concrete ones
| * | ||
| * @since 2.17 | ||
| */ | ||
| public boolean isEnabled(JsonWriteFeature f) { |
There was a problem hiding this comment.
Let's not add this here-- there is a reason it is missing (for legacy reasons). Basically, naming is bit confusing but JsonWriteFeature is strictly JSON-specific whereas JsonGenerator is not. So latter should not refer to former.
But we can add this new method instead in JsonGeneratorImpl base class, which is JSON-specific.
(alternative would be to do what code WriterBasedJsonGenerator currently does and instead use existing isEnabled(JsonGenerator.Feature), but that requires use of deprecated JsonGenerator.Feature constants).
| * @since 2.17 | ||
| */ | ||
| protected int[] escapeForwardSlash(int[] escapes) { | ||
| int[] esc = Arrays.copyOf(escapes, escapes.length); |
There was a problem hiding this comment.
I think this should be optimized a bit by checking if any change is actually needed -- and only making defensive copy in that case.
However, this leads to my main concern here: copying of the array for every generator instance adds overhead, which can be non-trivial in case of writing small documents.
So I wonder if there's any way to avoid this overhead by pre-creating escaping table in CharTypes.
|
Ok, I changed things I wanted to change. Looking at how escape-table is created, I don't think it is easy to try to make it happen by But it might still make sense to add an overload for |
|
@JooHyukKim Ok, I rewrote this a bit and I think it might be ready for merging. Thank you for providing the base & let me know if you think some changes are needed. Otherwise (or after changing) if you can convert it from draft to regular PR I can go ahead and merge it. Thank you again! |
|
@cowtowncoder seems good now 👍🏼 |
|
Thank you @JooHyukKim. I'll go ahead and merge this then. |
|
Follow-up wrt 3.0: #1200 |
resolves #507