Browse Source

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.
pull/2048/head
khazhyk 6 years ago
parent
commit
366dc4855b
  1. 12
      discord/abc.py
  2. 8
      discord/channel.py
  3. 37
      discord/iterators.py
  4. 4
      docs/locale/ja/LC_MESSAGES/api.po

12
discord/abc.py

@ -863,7 +863,7 @@ class Messageable(metaclass=abc.ABCMeta):
data = await state.http.pins_from(channel.id) data = await state.http.pins_from(channel.id)
return [state.create_message(channel=channel, data=m) for m in data] 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. """Return an :class:`.AsyncIterator` that enables receiving the destination's message history.
You must have :attr:`~.Permissions.read_message_history` permissions to use this. 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. 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 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. even number then this will return at most limit + 1 messages.
reverse: Optional[:class:`bool`] oldest_first: Optional[:class:`bool`]
If set to true, return messages in oldest->newest order. If unspecified, If set to true, return messages in oldest->newest order. Defaults to True if
this defaults to ``False`` for most cases. However if passing in a ``after`` is specified, otherwise False.
``after`` parameter then this is set to ``True``. This avoids getting messages
out of order in the ``after`` case.
Raises Raises
------ ------
@ -920,7 +918,7 @@ class Messageable(metaclass=abc.ABCMeta):
:class:`.Message` :class:`.Message`
The message with the message data parsed. 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): class Connectable(metaclass=abc.ABCMeta):

8
discord/channel.py

@ -265,7 +265,7 @@ class TextChannel(discord.abc.Messageable, discord.abc.GuildChannel, Hashable):
message_ids = [m.id for m in messages] message_ids = [m.id for m in messages]
await self._state.http.delete_messages(self.id, message_ids) 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| """|coro|
Purges a list of messages that meet the criteria given by the predicate 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`. Same as ``after`` in :meth:`history`.
around around
Same as ``around`` in :meth:`history`. Same as ``around`` in :meth:`history`.
reverse oldest_first
Same as ``reverse`` in :meth:`history`. Same as ``oldest_first`` in :meth:`history`.
bulk: class:`bool` bulk: class:`bool`
If True, use bulk delete. bulk=False is useful for mass-deleting If True, use bulk delete. bulk=False is useful for mass-deleting
a bot's own messages without manage_messages. When True, will fall 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: if check is None:
check = lambda m: True 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 = [] ret = []
count = 0 count = 0

37
discord/iterators.py

@ -28,10 +28,12 @@ import asyncio
import datetime import datetime
from .errors import NoMoreItems 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 .object import Object
from .audit_logs import AuditLogEntry from .audit_logs import AuditLogEntry
OLDEST_MESSAGE = Object(id=0)
class _AsyncIterator: class _AsyncIterator:
__slots__ = () __slots__ = ()
@ -196,14 +198,13 @@ class HistoryIterator(_AsyncIterator):
around: :class:`abc.Snowflake` around: :class:`abc.Snowflake`
Message around which all messages must be. Limit max 101. Note that if 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. limit is an even number, this will return at most limit+1 messages.
reverse: :class:`bool` oldest_first: :class:`bool`
If set to true, return messages in oldest->newest order. Recommended If set to true, return messages in oldest->newest order. Defaults to
when using with "after" queries with limit over 100, otherwise messages True if ``after`` is specified, otherwise False.
will be out of order.
""" """
def __init__(self, messageable, limit, 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): if isinstance(before, datetime.datetime):
before = Object(id=time_snowflake(before, high=False)) before = Object(id=time_snowflake(before, high=False))
@ -212,17 +213,17 @@ class HistoryIterator(_AsyncIterator):
if isinstance(around, datetime.datetime): if isinstance(around, datetime.datetime):
around = Object(id=time_snowflake(around)) 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.messageable = messageable
self.limit = limit self.limit = limit
self.before = before self.before = before
self.after = after self.after = after or OLDEST_MESSAGE
self.around = around self.around = around
if reverse is None:
self.reverse = after is not None
else:
self.reverse = reverse
self._filter = None # message dict -> bool self._filter = None # message dict -> bool
self.state = self.messageable._state self.state = self.messageable._state
@ -246,17 +247,15 @@ class HistoryIterator(_AsyncIterator):
self._filter = lambda m: int(m['id']) < self.before.id self._filter = lambda m: int(m['id']) < self.before.id
elif self.after: elif self.after:
self._filter = lambda m: self.after.id < int(m['id']) self._filter = lambda m: self.after.id < int(m['id'])
elif self.before and self.after: else:
if self.reverse: if self.reverse:
self._retrieve_messages = self._retrieve_messages_after_strategy 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: else:
self._retrieve_messages = self._retrieve_messages_before_strategy self._retrieve_messages = self._retrieve_messages_before_strategy
self._filter = lambda m: int(m['id']) > self.after.id if (self.after and self.after != OLDEST_MESSAGE):
elif self.after: self._filter = lambda m: int(m['id']) > self.after.id
self._retrieve_messages = self._retrieve_messages_after_strategy
else:
self._retrieve_messages = self._retrieve_messages_before_strategy
async def next(self): async def next(self):
if self.messages.empty(): if self.messages.empty():

4
docs/locale/ja/LC_MESSAGES/api.po

@ -7361,8 +7361,8 @@ msgid "Same as ``around`` in :meth:`history`."
msgstr ":meth:`history` での ``around`` と同じです。" msgstr ":meth:`history` での ``around`` と同じです。"
#: discord.TextChannel.purge:25 of #: discord.TextChannel.purge:25 of
msgid "Same as ``reverse`` in :meth:`history`." msgid "Same as ``oldest_first`` in :meth:`history`."
msgstr ":meth:`history` での ``reverse`` と同じです。" msgstr ":meth:`history` での ``oldest_first`` と同じです。"
#: discord.TextChannel.purge:26 of #: 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." 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."

Loading…
Cancel
Save