From 366dc4855b16422df0d22549820a5d5b87ef3c08 Mon Sep 17 00:00:00 2001 From: khazhyk Date: Sun, 7 Apr 2019 21:47:59 -0700 Subject: [PATCH] simplify HistoryIterator message ordering rename reverse -> oldest_first, which is more obvious what it does. Then, honor it entirely - if you specify no `after` endpoint, we default to the beginning of message history, similar to how `before` defaults to the end of message history. This is a breaking change, and will change the behavior of any iterator that previously would have been returning messages in a weird order for limits over 100 `for msg in history(reversed=True, limit=300)` would return the newest 300 messages, in a messed up order (100..0, 200..100, 300..200). `for msg in history(oldest_first=True, limit=300)` will now return the oldest 300 messages in order. And so on. `for msg in history(after=msg)` is unchanged, this previously would return the oldest 100 messages after `msg`, oldest->newest order, and still will. --- discord/abc.py | 12 +++++----- discord/channel.py | 8 +++---- discord/iterators.py | 37 +++++++++++++++---------------- docs/locale/ja/LC_MESSAGES/api.po | 4 ++-- 4 files changed, 29 insertions(+), 32 deletions(-) diff --git a/discord/abc.py b/discord/abc.py index 4b34e566b..86466d3da 100644 --- a/discord/abc.py +++ b/discord/abc.py @@ -863,7 +863,7 @@ class Messageable(metaclass=abc.ABCMeta): data = await state.http.pins_from(channel.id) return [state.create_message(channel=channel, data=m) for m in data] - def history(self, *, limit=100, before=None, after=None, around=None, reverse=None): + def history(self, *, limit=100, before=None, after=None, around=None, oldest_first=None): """Return an :class:`.AsyncIterator` that enables receiving the destination's message history. You must have :attr:`~.Permissions.read_message_history` permissions to use this. @@ -902,11 +902,9 @@ class Messageable(metaclass=abc.ABCMeta): If a date is provided it must be a timezone-naive datetime representing UTC time. When using this argument, the maximum limit is 101. Note that if the limit is an even number then this will return at most limit + 1 messages. - reverse: Optional[:class:`bool`] - If set to true, return messages in oldest->newest order. If unspecified, - this defaults to ``False`` for most cases. However if passing in a - ``after`` parameter then this is set to ``True``. This avoids getting messages - out of order in the ``after`` case. + oldest_first: Optional[:class:`bool`] + If set to true, return messages in oldest->newest order. Defaults to True if + ``after`` is specified, otherwise False. Raises ------ @@ -920,7 +918,7 @@ class Messageable(metaclass=abc.ABCMeta): :class:`.Message` The message with the message data parsed. """ - return HistoryIterator(self, limit=limit, before=before, after=after, around=around, reverse=reverse) + return HistoryIterator(self, limit=limit, before=before, after=after, around=around, oldest_first=oldest_first) class Connectable(metaclass=abc.ABCMeta): diff --git a/discord/channel.py b/discord/channel.py index 67b8981ce..86d93abf7 100644 --- a/discord/channel.py +++ b/discord/channel.py @@ -265,7 +265,7 @@ class TextChannel(discord.abc.Messageable, discord.abc.GuildChannel, Hashable): message_ids = [m.id for m in messages] await self._state.http.delete_messages(self.id, message_ids) - async def purge(self, *, limit=100, check=None, before=None, after=None, around=None, reverse=False, bulk=True): + async def purge(self, *, limit=100, check=None, before=None, after=None, around=None, oldest_first=False, bulk=True): """|coro| Purges a list of messages that meet the criteria given by the predicate @@ -306,8 +306,8 @@ class TextChannel(discord.abc.Messageable, discord.abc.GuildChannel, Hashable): Same as ``after`` in :meth:`history`. around Same as ``around`` in :meth:`history`. - reverse - Same as ``reverse`` in :meth:`history`. + oldest_first + Same as ``oldest_first`` in :meth:`history`. bulk: class:`bool` If True, use bulk delete. bulk=False is useful for mass-deleting a bot's own messages without manage_messages. When True, will fall @@ -330,7 +330,7 @@ class TextChannel(discord.abc.Messageable, discord.abc.GuildChannel, Hashable): if check is None: check = lambda m: True - iterator = self.history(limit=limit, before=before, after=after, reverse=reverse, around=around) + iterator = self.history(limit=limit, before=before, after=after, oldest_first=oldest_first, around=around) ret = [] count = 0 diff --git a/discord/iterators.py b/discord/iterators.py index 015620bca..73216ccff 100644 --- a/discord/iterators.py +++ b/discord/iterators.py @@ -28,10 +28,12 @@ import asyncio import datetime from .errors import NoMoreItems -from .utils import time_snowflake, maybe_coroutine +from .utils import DISCORD_EPOCH, time_snowflake, maybe_coroutine from .object import Object from .audit_logs import AuditLogEntry +OLDEST_MESSAGE = Object(id=0) + class _AsyncIterator: __slots__ = () @@ -196,14 +198,13 @@ class HistoryIterator(_AsyncIterator): around: :class:`abc.Snowflake` Message around which all messages must be. Limit max 101. Note that if limit is an even number, this will return at most limit+1 messages. - reverse: :class:`bool` - If set to true, return messages in oldest->newest order. Recommended - when using with "after" queries with limit over 100, otherwise messages - will be out of order. + oldest_first: :class:`bool` + If set to true, return messages in oldest->newest order. Defaults to + True if ``after`` is specified, otherwise False. """ def __init__(self, messageable, limit, - before=None, after=None, around=None, reverse=None): + before=None, after=None, around=None, oldest_first=None): if isinstance(before, datetime.datetime): before = Object(id=time_snowflake(before, high=False)) @@ -212,17 +213,17 @@ class HistoryIterator(_AsyncIterator): if isinstance(around, datetime.datetime): around = Object(id=time_snowflake(around)) + if oldest_first is None: + self.reverse = after is not None + else: + self.reverse = oldest_first + self.messageable = messageable self.limit = limit self.before = before - self.after = after + self.after = after or OLDEST_MESSAGE self.around = around - if reverse is None: - self.reverse = after is not None - else: - self.reverse = reverse - self._filter = None # message dict -> bool self.state = self.messageable._state @@ -246,17 +247,15 @@ class HistoryIterator(_AsyncIterator): self._filter = lambda m: int(m['id']) < self.before.id elif self.after: self._filter = lambda m: self.after.id < int(m['id']) - elif self.before and self.after: + else: if self.reverse: self._retrieve_messages = self._retrieve_messages_after_strategy - self._filter = lambda m: int(m['id']) < self.before.id + if (self.before): + self._filter = lambda m: int(m['id']) < self.before.id else: self._retrieve_messages = self._retrieve_messages_before_strategy - self._filter = lambda m: int(m['id']) > self.after.id - elif self.after: - self._retrieve_messages = self._retrieve_messages_after_strategy - else: - self._retrieve_messages = self._retrieve_messages_before_strategy + if (self.after and self.after != OLDEST_MESSAGE): + self._filter = lambda m: int(m['id']) > self.after.id async def next(self): if self.messages.empty(): diff --git a/docs/locale/ja/LC_MESSAGES/api.po b/docs/locale/ja/LC_MESSAGES/api.po index c947dbdac..f381f28d4 100644 --- a/docs/locale/ja/LC_MESSAGES/api.po +++ b/docs/locale/ja/LC_MESSAGES/api.po @@ -7361,8 +7361,8 @@ msgid "Same as ``around`` in :meth:`history`." msgstr ":meth:`history` での ``around`` と同じです。" #: discord.TextChannel.purge:25 of -msgid "Same as ``reverse`` in :meth:`history`." -msgstr ":meth:`history` での ``reverse`` と同じです。" +msgid "Same as ``oldest_first`` in :meth:`history`." +msgstr ":meth:`history` での ``oldest_first`` と同じです。" #: discord.TextChannel.purge:26 of msgid "If True, use bulk delete. bulk=False is useful for mass-deleting a bot's own messages without manage_messages. When True, will fall back to single delete if current account is a user bot, or if messages are older than two weeks."