Skip to content
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

pass escapeHTML flag through generated code #249

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jrmarkle
Copy link

@jrmarkle jrmarkle commented Oct 1, 2018

Possible fix for #237

@pquerna
Copy link
Owner

pquerna commented Oct 4, 2018

This is great, thank you!

My only concern is changing the marshalerFaster interface. I don't have an alternative though.

@jrmarkle
Copy link
Author

jrmarkle commented Oct 4, 2018

It's an internal interface but yeah, the only alternative I see is to add the flag to fflib.EncodingBuffer somehow and that seemed like a worse option.

@jrmarkle
Copy link
Author

can this get merged?

@dkegel-fastly
Copy link

I'm using

buf, err := ffjson.Marshal(&item)

and < is getting escaped in serialized values. I'd like to disable that, and this pull request looks apropos,
but it doesn't seem to expose the escapeHTML flag through to the user...?

@jrmarkle
Copy link
Author

jrmarkle commented Apr 25, 2021

I'm using

buf, err := ffjson.Marshal(&item)

and < is getting escaped in serialized values. I'd like to disable that, and this pull request looks apropos,
but it doesn't seem to expose the escapeHTML flag through to the user...?

IIRC it should work the same way as the standard library. You need to create an encoder and configure it. eg:

buf := new(bytes.Buffer)
enc := ffjson.NewEncoder(buf)
enc.SetEscapeHTML(false)
enc.Encode(item)

@dkegel-fastly
Copy link

Doesn't that lose the speed advantage of ffjson.Marshal()...?

@dkegel-fastly
Copy link

dkegel-fastly commented Apr 25, 2021

(I wonder if a generation-time flag might be a good idea... then you wouldn't have to thread that option through as much, the generated code would just have true or false hardcoded. It'd work for my use case. And it would avoid changing that interface...?)

@dkegel-fastly
Copy link

#260 makes it a little hard to play around with this...

@jrmarkle
Copy link
Author

Doesn't that lose the speed advantage of ffjson.Marshal()...?

Right, sorry. It should be ffjson.NewEncoder in the example (now corrected).

@dkegel-fastly
Copy link

dkegel-fastly commented Apr 25, 2021

I'm testing the change locally. To use my changed copy, I had to add this to go.mod in the app using ffjson:

replace github.com/pquerna/ffjson => /Users/dkegel/go/src/github.com/pquerna/ffjson

And it works! Well, kind of.

Oddly, even though I'm calling enc.SetEscapeHTML(false) both places I encode, in one case foo.MarshalJSON is being called, which hardcodes it to true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants