Conversation
Coverage report
This PR does not seem to contain any modification to coverable code. |
There was a problem hiding this comment.
Pull request overview
This PR addresses an issue with Discord message length limits by reducing the threshold from 1900 to 1800 characters in message length checks. The changes also include a minor style improvement to add proper spacing around the addition operator.
Changes:
- Reduced message length threshold from 1900 to 1800 characters in two length checks
- Fixed spacing around the
+operator to follow PEP 8 style guidelines
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for i, chunk in enumerate(chunks): | ||
| partial_message = f"\n\n## {header} ({i+1}/{len(chunks)}):\n" | ||
| partial_message = f"\n\n## {header} ({i + 1}/{len(chunks)}):\n" | ||
| partial_message += f"```\n{limit_length(chunk, 1900)}```" |
There was a problem hiding this comment.
The limit_length call still uses 1900 as the limit, which is inconsistent with the changed limits of 1800 in lines 111 and 123. This inconsistency could lead to messages exceeding the intended length limit. Consider updating this to 1800 to match the other changes, or adjust it to account for the header formatting overhead that's added in line 135.
| partial_message += f"```\n{limit_length(chunk, 1900)}```" | |
| partial_message += f"```\n{limit_length(chunk, 1800)}```" |
|
|
||
| async def _send_split_log(thread: discord.Thread, partial_message: str, header: str, log: str): | ||
| if len(partial_message) + len(log) + len(header) < 1900: | ||
| if len(partial_message) + len(log) + len(header) < 1800: |
There was a problem hiding this comment.
The length check on this line doesn't accurately account for the formatting overhead. The actual message will be partial_message + "\n\n## " + header + ":\n" + "```\n" + log + "```", which adds 14 extra characters (7 from the header formatting and 7 from the code block markers). The condition should be len(partial_message) + len(log) + len(header) + 14 < 1800 to accurately prevent messages from exceeding the limit.
| if len(partial_message) + len(log) + len(header) < 1800: | |
| if len(partial_message) + len(log) + len(header) + 14 < 1800: |
| partial_message = "" | ||
| for line in lines: | ||
| if len(partial_message) + len(line) < 1900: | ||
| if len(partial_message) + len(line) < 1800: |
There was a problem hiding this comment.
The length check doesn't account for the newline character that's added in line 124. The condition should be len(partial_message) + len(line) + 1 < 1800 to accurately represent the actual length after the newline is appended.
| if len(partial_message) + len(line) < 1800: | |
| if len(partial_message) + len(line) + 1 < 1800: |
1 participant
No description provided.