Illegal characters in file name or script are replaced by "!", and a … #967
Conversation
…warning emitted. Fixes increpare#932
| background_color=state.bgcolor; | ||
| } | ||
| htmlString = htmlString.replace(/___BGCOLOR___/g,background_color); | ||
| var title = state.metadata.title ? state.metadata.title : "PuzzleScript Game"; |
There was a problem hiding this comment.
In JavaScript nulls are typically coalesced like:
var title = state.metadata.title || "PuzzleScript Game";There was a problem hiding this comment.
That's my C/C++/C# showing up! I do know the JS way, but in so much of this code this would be written as an if/else with braces, I thought I was going out on a limb just making it a one-liner. But I approve the change.
| return safeDollar(nv); | ||
| } | ||
|
|
||
| // replace $ because it's special in replace |
There was a problem hiding this comment.
I don't understand this comment -- open to rework it / remove it.
Maybe it'd be clear enough to name the function more verbosely, like sanitizeDollarCharacter()?
(And similarly, for safeUser above, IMO, though it's a nitpick)
There was a problem hiding this comment.
The original comments was
// $ has special meaning to JavaScript's String.replace
so I was sticking somewhere near.
Again, happy to approve a change that makes it easier for the reader.
What are the mechanics to do that?
| htmlString = htmlString.replace(/"__GAMEDAT__"/g,sourceCode); | ||
| // $ has special meaning, so replace it by \v, then switch it back | ||
| htmlString = htmlString.replace(/"__GAMEDAT__"/, safeDollar(sourceCode)); | ||
| htmlString = htmlString.replace(/\v/g, '$$'); |
There was a problem hiding this comment.
I like the \v approach more than doubling all of our $s. Nice! 👍
| var BB = get_blob(); | ||
| var blob = new BB([htmlString], {type: "text/plain;charset=utf-8"}); | ||
| saveAs(blob, title+".html"); | ||
| saveAs(blob, fn); |
There was a problem hiding this comment.
IMO we could also replace the comment here with a more descriptive name than fn, like removeBadWindowsCharsFn
There was a problem hiding this comment.
I agree. But it's not a function, so maybe filename?
|
I had fixed that in a different way, by escaping the characters (hopefully) safely rather than replacing them, but forgot to close this issue. Thanks for the suggested change though! |
…warning emitted.
Fixes #932