search

Cog-Creators / Red-DiscordBot

A multi-function Discord bot
https://docs.discord.red
GNU General Public License v3.0
4.83k stars 2.3k forks source link

Make positive_int converter a public API #5939

Closed Jackenmen closed 1 year ago

Jackenmen commented 1 year ago

Type of feature request

API functionality

Description of the feature you're suggesting

Move positive_int converter (along with PositiveInt NewType) that's present in cleanup and economy cogs to redbot.core.commands.converter. Other than adding documentation, this should really only require moving things around.

Anything else?

This was suggested before here: https://github.com/Cog-Creators/Red-DiscordBot/pull/4583#issuecomment-725577831

Jackenmen commented 1 year ago

On the other hand, d.py now has a Range converter which could replace this entirely so maybe we shouldn't be adding such a converter... The current implementation of positive_int has a slightly better error message (which can be solved by special-casing Range[int, 0, None] in error handling) and is typed as NewType("PositiveInt") rather than plain int (this cannot be done with how Range is currently implemented) which isn't a lot.

The name does make it easier to comprehend so I think there's still a benefit to at least having the name available but maybe it should just be an alias for a pre-filled Range rather than a custom converter function.

Jackenmen commented 1 year ago

NewType is probably going to be a hindrance for type-checking more often than not so I guess it's not even worth bothering with it.