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

Escaped single quotes are not properly handled #168

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sdepold
Copy link

@sdepold sdepold commented May 14, 2015

The added test demonstrates an improper handling of escaped single quotes. Will see if I can get this fixed somehow.

If the line is changed from

throw new Error('Something hasn\'t been defined.');

to

throw new Error('Something has not been defined.');

it will work.

@sdepold
Copy link
Author

sdepold commented May 14, 2015

Apparently related to #149

@sdepold sdepold changed the title Single quotes are not properly handled Escaoed single quotes are not properly handled May 14, 2015
@sdepold sdepold changed the title Escaoed single quotes are not properly handled Escaped single quotes are not properly handled May 14, 2015
@shellscape
Copy link
Contributor

@tj please put some eyes on this one :)

@Twipped
Copy link
Collaborator

Twipped commented Oct 13, 2015

I think you meant me ;) i'll try to take a look at it this week.

On Oct 13, 2015, at 11:06 AM, Andrew Powell [email protected] wrote:

@tj please put some eyes on this one :)


Reply to this email directly or view it on GitHub.

@shellscape
Copy link
Contributor

@ChiperSoft thanks much!

@martindsouza
Copy link

@tj @shellscape @ChiperSoft I'm having the same issue as marked in this ticket: OraOpenSource/plsql-md-doc#21

It's due to single and double quote escaping in dox.js: } else if (!withinSingle && !withinMultiline && ('\'' == js[i] || '"' == js[i])) { Given how the files are parsed, I don't see how to get around it elegantly.

What do you think about providing an option called escapeQuotes which would default to true. Then change the above line to also include ... && escapeQuotes This would give the caller the option to disable this feature. It would then expose them to the risk of having /** in a quoted string triggering the multiLine mode however in most cases I think that's less likely than having an escaped single quote as @sdepold demonstrate above.

This suggestion is slightly different than @sdepold as it can handle for multiple languages. Oracle for instance, uses something called the q function to escape single quotes. Ex: q'! quote's!' would result in the string quote's. This can be very difficult to detect since the q function's escape character can be anything (in the example I provided I used ! but anything would do).

If you're ok with this I don't mind submitting the PR with this change.

_Note: could also call the parameter enableWithinString to match the variable. _

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.

4 participants