Synthoid / ExportSheetData

Add-on for Google Sheets that allows sheets to be exported as JSON or XML.
MIT License
236 stars 46 forks source link

Escape special HTML characters when visualizing instead of showing warning #60

Closed tomyam1 closed 6 years ago

tomyam1 commented 6 years ago

When visualizing JSON there a warning message about escaped characters:

Note: Escaped characters may not display properly when visualized, but will be properly formatted in the exported data.

A better approach would be to just escape the special HTML characters before passing them to the textarea, i.e.

& => &
> => >
< => &lt;

References: https://stackoverflow.com/a/4037287/1705056 https://stackoverflow.com/a/2502462/1705056

Will you consider a PR for this?

Thanks for this great tool, it says me lots and lots of time!

Synthoid commented 6 years ago

Hmm, interesting idea but I worry it may be confusing. Escaped characters in JSON are not escaped with HTML style char formatting and the escaped characters are much smaller in number than something like XML. For example:

Magic Pipes & You
A plumber's guide to saving the world

When exported to JSON, this becomes:

"Magic Pipes & You\nA plumber's guide to saving the world"

The reason for the note in the output window is that JSON escaped chars are read in as their standard versions in HTML pages. My concern with escaping chars with HTML style formatting may lead people to think their data will be formatted like that.

Synthoid commented 6 years ago

Closing this for now. If anyone would like to discuss it more or comes up with some more ideas, feel free to post and we can reopen the issue.

tomyam1 commented 6 years ago

Hey @Synthoid . Sorry for not responding earlier. I'll make sure to be responsive now.

I didn't quite understand your comment.

a. When exporting the text you gave, both the preview and opening the actual exported file give the same results. image image

b. When visualizing/exporting the following sheet:

image

You get:

image

image

If you escape the JSON as I suggested to:

{
  "Sheet2": [
    {
      "&amp;lt;&amp;nbsp;&amp;gt;": "&lt;/textarea&gt;"
    }
  ]
}

and set the text in the textarea to it, you get the correct result:

image

I don't see any downside to that.

Synthoid commented 6 years ago

Ah, I see what you're getting at! That absolutely makes sense. I thought you meant convert all JSON escaped characters to HTML escaped chars. This does complicate things though. Currently, the visualized text is the same text as the exported data. This change would require exporting data then converting that data to HTML escaped chars for the display area, correct?

tomyam1 commented 6 years ago

Created a PR that shows exactly what I suggest.

Note that I didn't test it though. Is it OK to ask that you'll test it yourself? (I don't have experience with writing GSuite extensions)

This change would require exporting data then converting that data to HTML escaped chars for the display area, correct? Yes. This is that the new escapeHtml function does

Do you think that the warning under the textarea is still needed?

Synthoid commented 6 years ago

The warning under the text area is still needed for JSON escaped characters, but your fixes should prevent weird HTML issues now. Thanks!

tomyam1 commented 6 years ago

Thanks a lot @Synthoid ! (Also for the link you mentioned to testing g suit scripts. If I'll make another PR I'll make sure to read it first)