Modify

Opened 17 years ago

Closed 13 years ago

Last modified 13 years ago

#2502 closed defect (fixed)

bad regexp.

Reported by: anonymous Owned by: Ryan J Ollos
Priority: normal Component: EmoticonsPlugin
Severity: normal Keywords:
Cc: Trac Release: 0.10

Description

You don't parenthesize your pipe-separated list of tokens, meaning the \b directives that you put at the beginning and end of the regexp only apply to the tokens at either end.

What you want is \b(abc|def|ghi|jkl)\b NOT \babc|def|ghi|jkl\b

-- or better yet \b(?:abc|def|ghi|jkl)\b

(note that the list will be sorted into some arbitrary order by python based upon it's hashing algorithm, the order you specify them in the EMOTICONS map is not relevant)

Basically, two of the emotes in your map will not be rendered, but which two is unpredictable.

With the above list, only the tokens 'def' and 'ghi' are successfully matched and rendered into emotes. abc and jkl are never matched and never rendered into emotes regardless of if they are surrounded by word boundaries or are substrings.

When I tried parenthesizing the expression (thus applying the \b directive to the beginning and end of EVERY token) I found that NOTHING is successfully matched.

Removing the \b directives appears to work just fine - all tokens are matched, and only if they are not substrings (in other words, there seems to be an implicit check for word boundaries anyway).

A quick check of the python tutorial shows that \b can also be considered the sequence for the backspace character. I don't (care to) know python very well, so I don't know if that's what you've done wrong, and I wasn't interested in trying to find out (whitespace sensitivity lol) so I'll leave the investigation into that possibility to you.

Without the \b directives, there is no longer any reason to parenthesize the expression either.

In short, your regexp seems mighty dodgy. This works just fine for me:

pattern = '|'.join([re.escape(pattern) for pattern in EMOTICONS]) yield pattern, _replace

Attachments (0)

Change History (4)

comment:1 Changed 14 years ago by Ryan J Ollos

Owner: changed from Christopher Lenz to Ryan J Ollos

comment:2 in reply to:  description Changed 13 years ago by Ryan J Ollos

Status: newassigned

Replying to anonymous:

Basically, two of the emotes in your map will not be rendered, but which two is unpredictable.

You are right, defect confirmed.

comment:3 Changed 13 years ago by Ryan J Ollos

Resolution: fixed
Status: assignedclosed

(In [10404]) Fixes #2502, #8723: Modified the regular expression to allow escaping and to fix issue such that two emoticons would not be matched.

comment:4 Changed 13 years ago by Ryan J Ollos

(In [10405]) Refs #2502, #8723: Remove print statement accidentally left in code.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.