Issue #82 solution #83
Conversation
…dle of the string
|
@kirsle Can you please take a review on this? |
rivescript/parser.py
Outdated
| return "Objects can only contain numbers and letters" | ||
| search = re.search(RE.name_syntax, line) | ||
| if search: | ||
| return "Objects can only contain numbers and letters" # TODO Acceptance of uppercase letter? |
There was a problem hiding this comment.
It seems safe enough to me to allow uppercase letters in object names. You may need to add a new syntax regexp to regexp.py to support that, like r'[^A-Za-z0-9_\-\s]'
There was a problem hiding this comment.
I add this accordingly.
tests/test_format_message.py
Outdated
| my ($rs, $args) = @_; | ||
| my $method = shift @{$args}; | ||
| <object | ||
| """) # Test for character violation in object, no % |
There was a problem hiding this comment.
I don't like how all these blocks of code are formatted. The indentations are all over the place.
Maybe something like this instead:
def test_invalid_character_raise_exception(self):
# This test passes with `match`, which only check at the beginning
self.assertRaises(Exception, self.new, """
+ $hello
- hi
""")
# etc for all the other tests.|
Just to be sure: this change makes the
|
|
Yes, $ is one of many invalid characters. The valid sets is ( | ) [ ] * _ # @ { } < > = and it is check at https://github.com/aichaos/rivescript-python/blob/master/rivescript/parser.py#L594. I may made a mistake putting a review there. I don't know how to quote the code to here. You can close that if not an appropriate place. |
|
@kirsle Do you plan to include this fix in the new release anytime soon? I find this bug fix is urgent because |
|
@Dinh-Hung-Tu I just published v1.14.6 to PyPI so you can update to get this fix. 😄 |
|
Thanks @kirsle! |
replace regex match by regex search for detection of invalid characters in the middle of the string.