Compact doc feature #3295
Compact doc feature #3295
Conversation
using a set to remove duplicates from the set of symbols ended up randomizing their order.
|
Sorry for the formatting changes to the unrelated lines. I think they're because I ran |
anand2312
requested review from
vivekashok1221,
ChrisLovering and
decorator-factory
Yeah, I think it's better not to change unrelated lines if we're not checking exactly matching |
| return ( | ||
| original.id == before.id | ||
| and before.content != after.content | ||
| and len(re.findall(COMPACT_DOC_REGEX, after.content.replace("`", ""))) > 0 |
There was a problem hiding this comment.
and re.search(COMPACT_DOC_REGEX, after.content.replace("`", "")) is not None| @@ -42,6 +44,10 @@ | |||
|
|
|||
| COMMAND_LOCK_SINGLETON = "inventory refresh" | |||
|
|
|||
| REDO_EMOJI = "\U0001f501" # :repeat: | |||
| REDO_TIMEOUT = 30 | |||
| COMPACT_DOC_REGEX = r"\[\[(?P<symbol>\S*)\]\]" | |||
There was a problem hiding this comment.
The reason I suggested matching specifically
[[`item`]]is that this won't match Python lists like [[]] or [[initial]] or [[1,2,3]] and lead to confusing/noisy messages from the bot. Maybe this could be changed to
r"\[\[`(?P<symbol>[^\s\]`]*)`\]\]"? This would also remove the need to strip ` from matches
| log.debug("Waiting for inventories to be refreshed before processing item.") | ||
| await self.refresh_event.wait() | ||
|
|
||
| # Ensure a refresh can't run in case of a context switch until the with block is exited |
There was a problem hiding this comment.
But a context switch cannot happen if there's no await/async for/async with
There was a problem hiding this comment.
I believe the code is fine but the comment needs adjusting. Not sure if context switch in the asyncio sense is what the original author (before anand moved it around) meant.
Ignore
| log.debug("Waiting for inventories to be refreshed before processing item.") | ||
| await self.refresh_event.wait() | ||
|
|
||
| # Ensure a refresh can't run in case of a context switch until the with block is exited |
There was a problem hiding this comment.
I believe the code is fine but the comment needs adjusting. Not sure if context switch in the asyncio sense is what the original author (before anand moved it around) meant.
Ignore
| @@ -439,8 +537,7 @@ async def refresh_command(self, ctx: commands.Context) -> None: | |||
| removed = "- " + removed | |||
|
|
|||
| embed = discord.Embed( | |||
| title="Inventories refreshed", | |||
| description=f"```diff\n{added}\n{removed}```" if added or removed else "" | |||
| title="Inventories refreshed", description=f"```diff\n{added}\n{removed}```" if added or removed else "" | |||
There was a problem hiding this comment.
You forgot to revert this formatting change.
As per @decorator-factory 's suggestion:
This PR implements the feature described, and also reacts to message edits and edits the bot's response.
The bot will only link up to 10 doc items, after which it ignores the rest.