Add Discord timestamp converter and transformer#10388
Add Discord timestamp converter and transformer#10388AbstractUmbra wants to merge 2 commits intoRapptz:masterfrom
Conversation
AbstractUmbra
left a comment
There was a problem hiding this comment.
Some of my own thoughts on it.
discord/ext/commands/converter.py
Outdated
| raise BadBoolArgument(lowered) | ||
|
|
||
|
|
||
| _TIMESTAMP_PATTERN: re.Pattern[str] = re.compile(r'<t:(\d+):[tTdDfFsSR]>') |
There was a problem hiding this comment.
Not keen on re-creating the same constant for both transformer and converter, but I wasn't sure where else I could put this to import from outside of utils.
| self.argument: str = argument | ||
| super().__init__(f'{argument} is not a recognised boolean option') | ||
|
|
||
| class BadDatetimeArgument(BadArgument): |
There was a problem hiding this comment.
As this uses the built-in datetime.datetime class, I opted for naming all resources DatetimeX, even thought this is more for timestamps than datetimes.
Feels unclean to me but naming this eludes me.
444cfba to
0bc6c97
Compare
|
A few suggestions for the implementation:
I’d also consider using |
9aee944 to
472f5aa
Compare
Good calls on those, thank you! 'twas an oversight on my part. None of the other error classes implement said slots however, so I'll opt out of adding those for now. |
472f5aa to
363e3b7
Compare
|
|
||
|
|
||
| def _convert_from_timestamp(argument: str) -> datetime.datetime: | ||
| match = _TIMESTAMP_PATTERN.match(argument) |
There was a problem hiding this comment.
Additionally I chose .match() over .search() since we'd expect to only parse the <t:xxxxxx:x> input only rather than search an arbitrary string.
Summary
This PR adds both a converter and transformer for use with Discord's timestamp functionality. This is made easier for the end user with the addition of the
@timeutility in the client, and as such I felt the need to support this.Checklist