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

Fix em and strong regex bug – failure to correctly recognise double nested escape sequences #430

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

Conversation

aidantwoods
Copy link
Collaborator

@aidantwoods aidantwoods commented Sep 23, 2016

Em and Strong regex were unable to recognise double nested escape sequences.
The previous strong regex didn't find the following to be a match
https://regex101.com/r/rH9oJ6/2
The updated regex recognises it
https://regex101.com/r/bC4xE5/4

Similarly, the em regex did not recognise this as a match
https://regex101.com/r/jS1lH9/2
But the updated regex again correctly matches
https://regex101.com/r/aU7oO1/2

Em and Strong regex were unable to recognise double nested escape sequences.
The previous strong regex didn't find the following to be a match
https://regex101.com/r/rH9oJ6/1
The updated regex recognises it
https://regex101.com/r/bC4xE5/1

Similarly, the em regex did not recognise this as a match
https://regex101.com/r/jS1lH9/1
But the updated regex again correctly matches
https://regex101.com/r/aU7oO1/1
@aidantwoods aidantwoods changed the title Update Parsedown.php Fix em and strong regex bug – failure to correctly recognise double nested escape sequences Sep 23, 2016
@rdelepine
Copy link

Not match this : **italic *bold\*it* italic** normal **italic**

@aidantwoods
Copy link
Collaborator Author

aidantwoods commented Sep 23, 2016

Was that match found with the code, or the link I gave?

Apologies on the link – I naively copy pasted the regex used directly, without removing the backslashes that PHP itself will treat as literal ;p

Link for that one should have been https://regex101.com/r/bC4xE5/4

I've updated the links in the original comment (including the ones testing the existing expressions)

@PhrozenByte
Copy link
Contributor

Unfortunately the regex considers just a single escape char, thus a escaped escape char isn't treaded correctly (i.e. you don't have to search for a single escape char, but any uneven number of escape chars). Thus the following is matched wrong:
https://regex101.com/r/3aZlVj/1

@aidantwoods
Copy link
Collaborator Author

@PhrozenByte I think you might be touching on a series of escaping problems within parsedown in various places ;p (#434)
I can address escapable escapes in the regexes above (e.g. this should fix the one you sent https://regex101.com/r/Y3rFDL/1), though this will probably be the first of many changes that need to be made to escaping

e.g.
screen shot 2016-10-04 at 21 46 39

@PhrozenByte
Copy link
Contributor

You have to start somewhere 😆 However, @erusev should decide whether this "escape escaping chars" issue should be addressed now or not.

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.

None yet

3 participants