Double quotes

[tdiam]

Even though double quotes were included in CHARS_GLOBAL_REGEXP there was no escape mapping.
This PR resolves this issue:

const tsqlstring = require('tsqlstring')

console.log(tsqlstring.escape('Sup"er'))
// before this PR: 'Supundefineder'
// after this PR: 'Sup\"er'

EDIT: Actually, if I understand it correctly, double quotes do not need escaping unless QUOTED_IDENTIFIER is ON.
@kylefarris how do you think this should be handled?

[samjross]

This fix doesn't work for me personally. I tested it out locally with an mssql server, and "\"" : '\\"', leaves me with a backslash in the actual string on the table.

For me, "\"" : '"', worked.

[samjross]

Seems to me that in mssql, " doesn't actually need to be escaped at all, so the real solution is probably to take \" out of CHARS_GLOBAL_REGEXP, but I'm not confident that I understand the purpose or scope if this library enough to test that. That's what works for me.

[kylefarris]

I think I went through this same conundrum when I was originally writing this library. In T-SQL the double-quote itself can be considered a way to protect identifiers but only if the setting is turned on for the server (something this library may not be able to easily detect). In other words, escaping the double-quote could potentially break queries that already have protected identifiers and cause it to totally break.

So, like @samjross said, I think we just need to remove the \" from the CHARS_GLOBAL_REGEXP (looks like removing that was just an oversight anyway).

So, @tdiam if you just want to make that change and undo the other stuff, that'd probably fix it. I'd be happy to get this PR merged in.

[kylefarris]

@tdiam I've updated your remote branch with some suggested edits. The only thing you need to do now is remove the double quote item from CHARS_GLOBAL_REGEXP.

[kylefarris]

@tdiam, actually, it looks like I was able to edit your branch directly somehow? So, I'll go ahead and merge this in and up date the package on NPM.

[kylefarris]

Patch added to NPM as v1.0.1.

[tdiam]

Yep, I reached the same conclusions. Thanks for the input and for merging @samjross, @kylefarris !

[samjross]

awesome, thank you @kylefarris !

[samjross]

@kylefarris I can't see a way to open an issue on this repo, but I've run into a problem with new lines, \n