From e9d350927365615d208d0ef4879d6205ffa97917 Mon Sep 17 00:00:00 2001 From: dgw Date: Thu, 25 Jan 2024 00:40:59 +0000 Subject: [PATCH 1/4] test: add basic tests for built-in `find` plugin Co-authored-by: Florian Strzelecki --- test/builtins/test_builtins_find.py | 60 +++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 test/builtins/test_builtins_find.py diff --git a/test/builtins/test_builtins_find.py b/test/builtins/test_builtins_find.py new file mode 100644 index 000000000..343803c24 --- /dev/null +++ b/test/builtins/test_builtins_find.py @@ -0,0 +1,60 @@ +"""Tests for Sopel's ``find`` plugin""" +from __future__ import annotations + +import pytest + +from sopel.formatting import bold +from sopel.tests import rawlist + + +TMP_CONFIG = """ +[core] +owner = Admin +nick = Sopel +enable = + find +host = irc.libera.chat +""" + + +@pytest.fixture +def bot(botfactory, configfactory): + settings = configfactory('default.ini', TMP_CONFIG) + return botfactory.preloaded(settings, ['find']) + + +@pytest.fixture +def irc(bot, ircfactory): + return ircfactory(bot) + + +@pytest.fixture +def user(userfactory): + return userfactory('User') + + +@pytest.fixture +def channel(): + return '#testing' + + +REPLACES_THAT_WORK = ( + ("A simple line.", r"s/line/message/", f"A simple {bold('message')}."), + ("An escaped / line.", r"s/\//slash/", f"An escaped {bold('slash')} line."), + ("A piped line.", r"s|line|replacement|", f"A piped {bold('replacement')}."), + ("An escaped | line.", r"s|\||pipe|", f"An escaped {bold('pipe')} line."), +) + + +@pytest.mark.parametrize('original, command, result', REPLACES_THAT_WORK) +def test_valid_replacements(bot, irc, user, channel, original, command, result): + irc.channel_joined(channel, [user.nick]) + + irc.say(user, channel, original) + irc.say(user, channel, command) + + assert len(bot.backend.message_sent) == 1, ( + "The bot should respond with exactly one line.") + assert bot.backend.message_sent == rawlist( + "PRIVMSG %s :%s meant to say: %s" % (channel, user.nick, result), + ) From d4cacab47930c1b2f2fb55196fac7df1bc846ddf Mon Sep 17 00:00:00 2001 From: dgw Date: Thu, 25 Jan 2024 01:35:08 +0000 Subject: [PATCH 2/4] find: support double-backslash for literal backslash in search pattern --- sopel/builtins/find.py | 23 +++++++++++++++++------ test/builtins/test_builtins_find.py | 1 + 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/sopel/builtins/find.py b/sopel/builtins/find.py index db6528252..b51b3f957 100644 --- a/sopel/builtins/find.py +++ b/sopel/builtins/find.py @@ -121,11 +121,11 @@ def kick_cleanup(bot, trigger): [:,]\s+)? # Followed by optional colon/comma and whitespace s(?P/) # The literal s and a separator / as group 2 (?P # Group 3 is the thing to find - (?:\\/|[^/])+ # One or more non-slashes or escaped slashes + (?:\\\\|\\/|[^/])+ # One or more non-slashes or escaped slashes ) / # The separator again (?P # Group 4 is what to replace with - (?:\\/|[^/])* # One or more non-slashes or escaped slashes + (?:\\\\|\\/|[^/])* # One or more non-slashes or escaped slashes ) (?:/ # Optional separator followed by group 5 (flags) (?P\S+) @@ -136,11 +136,11 @@ def kick_cleanup(bot, trigger): [:,]\s+)? # Followed by optional colon/comma and whitespace s(?P\|) # The literal s and a separator | as group 2 (?P # Group 3 is the thing to find - (?:\\\||[^|])+ # One or more non-pipe or escaped pipe + (?:\\\\|\\\||[^|])+ # One or more non-pipe or escaped pipe ) \| # The separator again (?P # Group 4 is what to replace with - (?:\\\||[^|])* # One or more non-pipe or escaped pipe + (?:\\\\|\\\||[^|])* # One or more non-pipe or escaped pipe ) (?:\| # Optional separator followed by group 5 (flags) (?P\S+) @@ -161,14 +161,16 @@ def findandreplace(bot, trigger): return sep = trigger.group('sep') - old = trigger.group('old').replace('\\%s' % sep, sep) + escape_sequence_pattern = re.compile(r'\\[\\%s]' % sep) + + old = escape_sequence_pattern.sub(decode_escape, trigger.group('old')) new = trigger.group('new') me = False # /me command flags = trigger.group('flags') or '' # only clean/format the new string if it's non-empty if new: - new = bold(new.replace('\\%s' % sep, sep)) + new = bold(escape_sequence_pattern.sub(decode_escape, new)) # If g flag is given, replace all. Otherwise, replace once. if 'g' in flags: @@ -217,3 +219,12 @@ def repl(s): phrase = '%s %s' % (trigger.nick, new_phrase) bot.say(phrase) + + +def decode_escape(match): + print("Substituting %s" % match.group(0)) + return { + r'\\': '\\', + r'\|': '|', + r'\/': '/', + }[match.group(0)] diff --git a/test/builtins/test_builtins_find.py b/test/builtins/test_builtins_find.py index 343803c24..1cc3b5ce2 100644 --- a/test/builtins/test_builtins_find.py +++ b/test/builtins/test_builtins_find.py @@ -43,6 +43,7 @@ def channel(): ("An escaped / line.", r"s/\//slash/", f"An escaped {bold('slash')} line."), ("A piped line.", r"s|line|replacement|", f"A piped {bold('replacement')}."), ("An escaped | line.", r"s|\||pipe|", f"An escaped {bold('pipe')} line."), + ("An escaped \\ line.", r"s/\\/backslash/", f"An escaped {bold('backslash')} line."), ) From 04a5b43cc60cefaf7e85849c83967e25db630a28 Mon Sep 17 00:00:00 2001 From: dgw Date: Sun, 1 Sep 2024 23:10:40 -0500 Subject: [PATCH 3/4] test: verify more `find` behaviors Check that correcting someone else's line works as expected, and check that replacing an already-replaced line works as expected. Also check that case-insensitive (i) and global (g) flags work as expected, both separately and together. Note: There is a glitch in formatting when the new replacement occurs inside the previous one, since the bolding used to indicate what changed is kept in the stored line. We might wish to fix that at some point, but the new test correctly represents the real behavior. --- test/builtins/test_builtins_find.py | 48 +++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/builtins/test_builtins_find.py b/test/builtins/test_builtins_find.py index 1cc3b5ce2..fee09f6c4 100644 --- a/test/builtins/test_builtins_find.py +++ b/test/builtins/test_builtins_find.py @@ -33,6 +33,11 @@ def user(userfactory): return userfactory('User') +@pytest.fixture +def other_user(userfactory): + return userfactory('other_user') + + @pytest.fixture def channel(): return '#testing' @@ -44,11 +49,15 @@ def channel(): ("A piped line.", r"s|line|replacement|", f"A piped {bold('replacement')}."), ("An escaped | line.", r"s|\||pipe|", f"An escaped {bold('pipe')} line."), ("An escaped \\ line.", r"s/\\/backslash/", f"An escaped {bold('backslash')} line."), + ("abABab", r"s/b/c/g", "abABab".replace('b', bold('c'))), # g (global) flag + ("ABabAB", r"s/b/c/i", f"A{bold('c')}abAB"), # i (case-insensitive) flag + ("ABabAB", r"s/b/c/ig", f"A{bold('c')}a{bold('c')}A{bold('c')}"), # both flags ) @pytest.mark.parametrize('original, command, result', REPLACES_THAT_WORK) def test_valid_replacements(bot, irc, user, channel, original, command, result): + """Verify that basic replacement functionality works.""" irc.channel_joined(channel, [user.nick]) irc.say(user, channel, original) @@ -59,3 +68,42 @@ def test_valid_replacements(bot, irc, user, channel, original, command, result): assert bot.backend.message_sent == rawlist( "PRIVMSG %s :%s meant to say: %s" % (channel, user.nick, result), ) + + +def test_multiple_users(bot, irc, user, other_user, channel): + """Verify that correcting another user's line works.""" + irc.channel_joined(channel, [user.nick, other_user.nick]) + + irc.say(other_user, channel, 'Some weather we got yesterday') + irc.say(user, channel, '%s: s/yester/to/' % other_user.nick) + + assert len(bot.backend.message_sent) == 1, ( + "The bot should respond with exactly one line.") + assert bot.backend.message_sent == rawlist( + "PRIVMSG %s :%s thinks %s meant to say: %s" % ( + channel, user.nick, other_user.nick, + f"Some weather we got {bold('to')}day", + ), + ) + + +def test_replace_the_replacement(bot, irc, user, channel): + """Verify replacing text that was already replaced.""" + irc.channel_joined(channel, [user.nick]) + + irc.say(user, channel, 'spam') + irc.say(user, channel, 's/spam/eggs/') + irc.say(user, channel, 's/eggs/bacon/') + + assert len(bot.backend.message_sent) == 2, ( + "The bot should respond twice.") + assert bot.backend.message_sent == rawlist( + "PRIVMSG %s :%s meant to say: %s" % ( + channel, user.nick, bold('eggs'), + ), + "PRIVMSG %s :%s meant to say: %s" % ( + channel, user.nick, bold(bold('bacon')), + # the test is accurate, even though the behavior here (doubled bold + # control characters) is less than ideal + ), + ) From 18106f84cedea757a19178f8f36fcff140ec49d9 Mon Sep 17 00:00:00 2001 From: dgw Date: Sun, 1 Sep 2024 23:33:00 -0500 Subject: [PATCH 4/4] find: fix bolding when replacing inside a previous replacement Perform the replacement with an unformatted version of the substitute string, then create a display version with bolding added only after the matching line (if any) is found. Updated the corresponding test case. Also renamed some local vars in the `find` plugin file to make a bit more sense (but not too dramatic). --- sopel/builtins/find.py | 29 +++++++++++++++-------------- test/builtins/test_builtins_find.py | 4 +--- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/sopel/builtins/find.py b/sopel/builtins/find.py index b51b3f957..67721d593 100644 --- a/sopel/builtins/find.py +++ b/sopel/builtins/find.py @@ -170,7 +170,7 @@ def findandreplace(bot, trigger): # only clean/format the new string if it's non-empty if new: - new = bold(escape_sequence_pattern.sub(decode_escape, new)) + new = escape_sequence_pattern.sub(decode_escape, new) # If g flag is given, replace all. Otherwise, replace once. if 'g' in flags: @@ -183,42 +183,43 @@ def findandreplace(bot, trigger): if 'i' in flags: regex = re.compile(re.escape(old), re.U | re.I) - def repl(s): - return re.sub(regex, new, s, count == 1) + def repl(line, subst): + return re.sub(regex, subst, line, count == 1) else: - def repl(s): - return s.replace(old, new, count) + def repl(line, subst): + return line.replace(old, subst, count) # Look back through the user's lines in the channel until you find a line # where the replacement works - new_phrase = None + new_line = new_display = None for line in history: if line.startswith("\x01ACTION"): me = True # /me command line = line[8:] else: me = False - replaced = repl(line) + replaced = repl(line, new) if replaced != line: # we are done - new_phrase = replaced + new_line = replaced + new_display = repl(line, bold(new)) break - if not new_phrase: + if not new_line: return # Didn't find anything # Save the new "edited" message. action = (me and '\x01ACTION ') or '' # If /me message, prepend \x01ACTION - history.appendleft(action + new_phrase) # history is in most-recent-first order + history.appendleft(action + new_line) # history is in most-recent-first order # output if not me: - new_phrase = 'meant to say: %s' % new_phrase + new_display = 'meant to say: %s' % new_display if trigger.group(1): - phrase = '%s thinks %s %s' % (trigger.nick, rnick, new_phrase) + msg = '%s thinks %s %s' % (trigger.nick, rnick, new_display) else: - phrase = '%s %s' % (trigger.nick, new_phrase) + msg = '%s %s' % (trigger.nick, new_display) - bot.say(phrase) + bot.say(msg) def decode_escape(match): diff --git a/test/builtins/test_builtins_find.py b/test/builtins/test_builtins_find.py index fee09f6c4..f7daaa1f2 100644 --- a/test/builtins/test_builtins_find.py +++ b/test/builtins/test_builtins_find.py @@ -102,8 +102,6 @@ def test_replace_the_replacement(bot, irc, user, channel): channel, user.nick, bold('eggs'), ), "PRIVMSG %s :%s meant to say: %s" % ( - channel, user.nick, bold(bold('bacon')), - # the test is accurate, even though the behavior here (doubled bold - # control characters) is less than ideal + channel, user.nick, bold('bacon'), ), )