From 4d22554396e326539b05ac65a83b844a8dec965c Mon Sep 17 00:00:00 2001 From: Ionite Date: Tue, 25 Oct 2022 00:45:08 -0400 Subject: [PATCH 01/10] Temporary fix for greedy parsing freezing --- bot/exts/info/information.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index 2592e093d0..088d0af6e1 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -524,13 +524,16 @@ async def json(self, ctx: Context, message: Message) -> None: await self.send_raw_content(ctx, message, json=True) @command(aliases=("rule",)) - async def rules(self, ctx: Context, *args: Optional[str]) -> Optional[Set[int]]: + async def rules(self, ctx: Context, arg: Optional[str]) -> Optional[Set[int]]: """ Provides a link to all rules or, if specified, displays specific rule(s). It accepts either rule numbers or particular keywords that map to a particular rule. Rule numbers and keywords can be sent in any order. """ + # Temporary fix for discord.py greedy string quote conversion bug + args = (arg or "",) + rules_embed = Embed(title="Rules", color=Colour.og_blurple(), url="https://www.pythondiscord.com/pages/rules") keywords, rule_numbers = [], [] From 461c7fff23384187add4e5fa0eb37f5b0721098f Mon Sep 17 00:00:00 2001 From: Ionite Date: Tue, 25 Oct 2022 01:01:56 -0400 Subject: [PATCH 02/10] Made arg fix compatible with tests --- bot/exts/info/information.py | 5 +++-- tests/bot/exts/info/test_information.py | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index 088d0af6e1..5aa721a01b 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -524,7 +524,7 @@ async def json(self, ctx: Context, message: Message) -> None: await self.send_raw_content(ctx, message, json=True) @command(aliases=("rule",)) - async def rules(self, ctx: Context, arg: Optional[str]) -> Optional[Set[int]]: + async def rules(self, ctx: Context, args: Optional[str]) -> Optional[Set[int]]: """ Provides a link to all rules or, if specified, displays specific rule(s). @@ -532,7 +532,8 @@ async def rules(self, ctx: Context, arg: Optional[str]) -> Optional[Set[int]]: Rule numbers and keywords can be sent in any order. """ # Temporary fix for discord.py greedy string quote conversion bug - args = (arg or "",) + if not args: + args = ("",) rules_embed = Embed(title="Rules", color=Colour.og_blurple(), url="https://www.pythondiscord.com/pages/rules") keywords, rule_numbers = [], [] diff --git a/tests/bot/exts/info/test_information.py b/tests/bot/exts/info/test_information.py index 9f5143c012..267346c58d 100644 --- a/tests/bot/exts/info/test_information.py +++ b/tests/bot/exts/info/test_information.py @@ -614,7 +614,7 @@ async def test_return_none_if_one_rule_number_is_invalid(self): str(rule_number) for rule_number in extracted_rule_numbers if rule_number < 1 or rule_number > len(self.full_rules)) - final_rule_numbers = await self.cog.rules(self.cog, self.ctx, *raw_user_input) + final_rule_numbers = await self.cog.rules(self.cog, self.ctx, raw_user_input) self.assertEqual( self.ctx.send.call_args, @@ -631,7 +631,7 @@ async def test_return_correct_rule_numbers(self): for raw_user_input, expected_matched_rule_numbers in test_cases: with self.subTest(identifier=raw_user_input): - final_rule_numbers = await self.cog.rules(self.cog, self.ctx, *raw_user_input) + final_rule_numbers = await self.cog.rules(self.cog, self.ctx, raw_user_input) self.assertEqual(expected_matched_rule_numbers, final_rule_numbers) async def test_return_default_rules_when_no_input_or_no_match_are_found(self): @@ -643,7 +643,7 @@ async def test_return_default_rules_when_no_input_or_no_match_are_found(self): for raw_user_input, expected_matched_rule_numbers in test_cases: with self.subTest(identifier=raw_user_input): - final_rule_numbers = await self.cog.rules(self.cog, self.ctx, *raw_user_input) + final_rule_numbers = await self.cog.rules(self.cog, self.ctx, raw_user_input) embed = self.ctx.send.call_args.kwargs['embed'] self.assertEqual(information.DEFAULT_RULES_DESCRIPTION, embed.description) self.assertEqual(expected_matched_rule_numbers, final_rule_numbers) From 1a4a147ebe6ce8529e811707dcf545e3393cc29a Mon Sep 17 00:00:00 2001 From: Ionite Date: Tue, 25 Oct 2022 01:23:20 -0400 Subject: [PATCH 03/10] Updated fix --- bot/exts/info/information.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index 5aa721a01b..80e7a5b6c8 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -533,7 +533,13 @@ async def rules(self, ctx: Context, args: Optional[str]) -> Optional[Set[int]]: """ # Temporary fix for discord.py greedy string quote conversion bug if not args: - args = ("",) + args = () + elif isinstance(args, str): + msg = ctx.message.content + # Remove the command + if len(msg := msg.split()) > 1: + msg.pop(0) + args = tuple(msg) rules_embed = Embed(title="Rules", color=Colour.og_blurple(), url="https://www.pythondiscord.com/pages/rules") keywords, rule_numbers = [], [] From 54e3fcff5411b9026d4a31759842aae25611859e Mon Sep 17 00:00:00 2001 From: Ionite Date: Tue, 25 Oct 2022 09:47:12 -0400 Subject: [PATCH 04/10] Added new star expression fix --- bot/exts/info/information.py | 28 ++++++++++--------------- tests/bot/exts/info/test_information.py | 6 +++--- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index 80e7a5b6c8..73afa33090 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -524,22 +524,15 @@ async def json(self, ctx: Context, message: Message) -> None: await self.send_raw_content(ctx, message, json=True) @command(aliases=("rule",)) - async def rules(self, ctx: Context, args: Optional[str]) -> Optional[Set[int]]: + async def rules(self, ctx: Context, *, args: Optional[str]) -> Optional[Set[int]]: """ Provides a link to all rules or, if specified, displays specific rule(s). It accepts either rule numbers or particular keywords that map to a particular rule. Rule numbers and keywords can be sent in any order. """ - # Temporary fix for discord.py greedy string quote conversion bug - if not args: - args = () - elif isinstance(args, str): - msg = ctx.message.content - # Remove the command - if len(msg := msg.split()) > 1: - msg.pop(0) - args = tuple(msg) + # Note: *args cannot be used due to a discord.py bug + # causing infinite loops during greedy string parsing. rules_embed = Embed(title="Rules", color=Colour.og_blurple(), url="https://www.pythondiscord.com/pages/rules") keywords, rule_numbers = [], [] @@ -551,13 +544,14 @@ async def rules(self, ctx: Context, args: Optional[str]) -> Optional[Set[int]]: for rule_keyword in rule_keywords: keyword_to_rule_number[rule_keyword] = rule_number - for word in args: - try: - rule_numbers.append(int(word)) - except ValueError: - if (kw := word.lower()) not in keyword_to_rule_number: - break - keywords.append(kw) + if args: + for word in args.split(): + try: + rule_numbers.append(int(word)) + except ValueError: + if (kw := word.lower()) not in keyword_to_rule_number: + break + keywords.append(kw) if not rule_numbers and not keywords: # Neither rules nor keywords were submitted. Return the default description. diff --git a/tests/bot/exts/info/test_information.py b/tests/bot/exts/info/test_information.py index 267346c58d..9f5143c012 100644 --- a/tests/bot/exts/info/test_information.py +++ b/tests/bot/exts/info/test_information.py @@ -614,7 +614,7 @@ async def test_return_none_if_one_rule_number_is_invalid(self): str(rule_number) for rule_number in extracted_rule_numbers if rule_number < 1 or rule_number > len(self.full_rules)) - final_rule_numbers = await self.cog.rules(self.cog, self.ctx, raw_user_input) + final_rule_numbers = await self.cog.rules(self.cog, self.ctx, *raw_user_input) self.assertEqual( self.ctx.send.call_args, @@ -631,7 +631,7 @@ async def test_return_correct_rule_numbers(self): for raw_user_input, expected_matched_rule_numbers in test_cases: with self.subTest(identifier=raw_user_input): - final_rule_numbers = await self.cog.rules(self.cog, self.ctx, raw_user_input) + final_rule_numbers = await self.cog.rules(self.cog, self.ctx, *raw_user_input) self.assertEqual(expected_matched_rule_numbers, final_rule_numbers) async def test_return_default_rules_when_no_input_or_no_match_are_found(self): @@ -643,7 +643,7 @@ async def test_return_default_rules_when_no_input_or_no_match_are_found(self): for raw_user_input, expected_matched_rule_numbers in test_cases: with self.subTest(identifier=raw_user_input): - final_rule_numbers = await self.cog.rules(self.cog, self.ctx, raw_user_input) + final_rule_numbers = await self.cog.rules(self.cog, self.ctx, *raw_user_input) embed = self.ctx.send.call_args.kwargs['embed'] self.assertEqual(information.DEFAULT_RULES_DESCRIPTION, embed.description) self.assertEqual(expected_matched_rule_numbers, final_rule_numbers) From 71fdd6cfd4e5353be61a46a2ab629835238122fc Mon Sep 17 00:00:00 2001 From: Ionite Date: Tue, 25 Oct 2022 09:53:47 -0400 Subject: [PATCH 05/10] Update tests for new single string format --- tests/bot/exts/info/test_information.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/bot/exts/info/test_information.py b/tests/bot/exts/info/test_information.py index 9f5143c012..e016ae8edc 100644 --- a/tests/bot/exts/info/test_information.py +++ b/tests/bot/exts/info/test_information.py @@ -603,9 +603,9 @@ def setUp(self) -> None: async def test_return_none_if_one_rule_number_is_invalid(self): test_cases = [ - (('1', '6', '7', '8'), (6, 7, 8)), - (('10', "first"), (10, )), - (("first", 10), (10, )) + ("1 6 7 8", (6, 7, 8)), + ("10 first", (10,)), + ("first 10", (10,)) ] for raw_user_input, extracted_rule_numbers in test_cases: @@ -614,7 +614,7 @@ async def test_return_none_if_one_rule_number_is_invalid(self): str(rule_number) for rule_number in extracted_rule_numbers if rule_number < 1 or rule_number > len(self.full_rules)) - final_rule_numbers = await self.cog.rules(self.cog, self.ctx, *raw_user_input) + final_rule_numbers = await self.cog.rules(self.cog, self.ctx, args=raw_user_input) self.assertEqual( self.ctx.send.call_args, @@ -624,26 +624,26 @@ async def test_return_none_if_one_rule_number_is_invalid(self): async def test_return_correct_rule_numbers(self): test_cases = [ - (("1", "2", "first"), {1, 2}), - (("1", "hello", "2", "second"), {1}), - (("second", "third", "unknown", "999"), {2, 3}) + ("1 2 first", {1, 2}), + ("1 hello 2 second", {1}), + ("second third unknown 999", {2, 3}) ] for raw_user_input, expected_matched_rule_numbers in test_cases: with self.subTest(identifier=raw_user_input): - final_rule_numbers = await self.cog.rules(self.cog, self.ctx, *raw_user_input) + final_rule_numbers = await self.cog.rules(self.cog, self.ctx, args=raw_user_input) self.assertEqual(expected_matched_rule_numbers, final_rule_numbers) async def test_return_default_rules_when_no_input_or_no_match_are_found(self): test_cases = [ - ((), None), - (("hello", "2", "second"), None), - (("hello", "999"), None), + ("", None), + ("hello 2 second", None), + ("hello 999", None), ] for raw_user_input, expected_matched_rule_numbers in test_cases: with self.subTest(identifier=raw_user_input): - final_rule_numbers = await self.cog.rules(self.cog, self.ctx, *raw_user_input) + final_rule_numbers = await self.cog.rules(self.cog, self.ctx, args=raw_user_input) embed = self.ctx.send.call_args.kwargs['embed'] self.assertEqual(information.DEFAULT_RULES_DESCRIPTION, embed.description) self.assertEqual(expected_matched_rule_numbers, final_rule_numbers) From c5c8e3d399654b18d8dee1cb43626b770032930f Mon Sep 17 00:00:00 2001 From: Ionite Date: Tue, 25 Oct 2022 10:35:27 -0400 Subject: [PATCH 06/10] Remove comment, arg parse bug --- bot/exts/info/information.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index 73afa33090..eb1983c074 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -531,9 +531,6 @@ async def rules(self, ctx: Context, *, args: Optional[str]) -> Optional[Set[int] It accepts either rule numbers or particular keywords that map to a particular rule. Rule numbers and keywords can be sent in any order. """ - # Note: *args cannot be used due to a discord.py bug - # causing infinite loops during greedy string parsing. - rules_embed = Embed(title="Rules", color=Colour.og_blurple(), url="https://www.pythondiscord.com/pages/rules") keywords, rule_numbers = [], [] From c0e9c54af1cd0d8f2c24bd5f3a1f944c96ab29d0 Mon Sep 17 00:00:00 2001 From: Ionite Date: Tue, 25 Oct 2022 10:41:27 -0400 Subject: [PATCH 07/10] Parse all words instead of breaking Co-authored-by: ChrisJL --- bot/exts/info/information.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index 73afa33090..61ed637711 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -549,9 +549,8 @@ async def rules(self, ctx: Context, *, args: Optional[str]) -> Optional[Set[int] try: rule_numbers.append(int(word)) except ValueError: - if (kw := word.lower()) not in keyword_to_rule_number: - break - keywords.append(kw) + if (kw := word.lower()) in keyword_to_rule_number: + keywords.append(kw) if not rule_numbers and not keywords: # Neither rules nor keywords were submitted. Return the default description. From 884d8ec47abddccbc83aa33749939c179e1b04ea Mon Sep 17 00:00:00 2001 From: Ionite Date: Tue, 25 Oct 2022 10:45:06 -0400 Subject: [PATCH 08/10] Add args split max limit --- bot/exts/info/information.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index bb06af0ab2..44cd55362e 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -542,7 +542,7 @@ async def rules(self, ctx: Context, *, args: Optional[str]) -> Optional[Set[int] keyword_to_rule_number[rule_keyword] = rule_number if args: - for word in args.split(): + for word in args.split(maxsplit=100): try: rule_numbers.append(int(word)) except ValueError: From 8023741e77394877cafda526fc2cbca50aaa77db Mon Sep 17 00:00:00 2001 From: Ionite Date: Tue, 25 Oct 2022 10:56:13 -0400 Subject: [PATCH 09/10] Update tests to work with rules arg fix --- tests/bot/exts/info/test_information.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/bot/exts/info/test_information.py b/tests/bot/exts/info/test_information.py index e016ae8edc..0444ca465e 100644 --- a/tests/bot/exts/info/test_information.py +++ b/tests/bot/exts/info/test_information.py @@ -625,8 +625,8 @@ async def test_return_correct_rule_numbers(self): test_cases = [ ("1 2 first", {1, 2}), - ("1 hello 2 second", {1}), - ("second third unknown 999", {2, 3}) + ("1 hello 2 second", {1, 2}), + ("second third unknown 999", None) ] for raw_user_input, expected_matched_rule_numbers in test_cases: @@ -637,8 +637,7 @@ async def test_return_correct_rule_numbers(self): async def test_return_default_rules_when_no_input_or_no_match_are_found(self): test_cases = [ ("", None), - ("hello 2 second", None), - ("hello 999", None), + ("hello abc", None), ] for raw_user_input, expected_matched_rule_numbers in test_cases: From 8f68607654e92e1ff1c026d37f339a983ebe8840 Mon Sep 17 00:00:00 2001 From: Ionite Date: Tue, 25 Oct 2022 11:19:05 -0400 Subject: [PATCH 10/10] Reverse changes to invalid arg break --- bot/exts/info/information.py | 6 ++++-- tests/bot/exts/info/test_information.py | 7 ++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index 44cd55362e..2eb9382e36 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -546,8 +546,10 @@ async def rules(self, ctx: Context, *, args: Optional[str]) -> Optional[Set[int] try: rule_numbers.append(int(word)) except ValueError: - if (kw := word.lower()) in keyword_to_rule_number: - keywords.append(kw) + # Stop on first invalid keyword/index to allow for normal messaging after + if (kw := word.lower()) not in keyword_to_rule_number: + break + keywords.append(kw) if not rule_numbers and not keywords: # Neither rules nor keywords were submitted. Return the default description. diff --git a/tests/bot/exts/info/test_information.py b/tests/bot/exts/info/test_information.py index 0444ca465e..65595e9592 100644 --- a/tests/bot/exts/info/test_information.py +++ b/tests/bot/exts/info/test_information.py @@ -625,8 +625,8 @@ async def test_return_correct_rule_numbers(self): test_cases = [ ("1 2 first", {1, 2}), - ("1 hello 2 second", {1, 2}), - ("second third unknown 999", None) + ("1 hello 2 second", {1}), + ("second third unknown 999", {2, 3}), ] for raw_user_input, expected_matched_rule_numbers in test_cases: @@ -637,7 +637,8 @@ async def test_return_correct_rule_numbers(self): async def test_return_default_rules_when_no_input_or_no_match_are_found(self): test_cases = [ ("", None), - ("hello abc", None), + ("hello 2 second", None), + ("hello 999", None), ] for raw_user_input, expected_matched_rule_numbers in test_cases: