Skip to content

Conversation

tim-schilling
Copy link

This is another attempt at #42

I squashed the commits, rebased on main, and resolved most of the mypy issues. The Plugin concerns already exist in nag, so I'm not sure how to handle those.

I also pinned the new dependency to a specific commit to help avoid security issues down the road. However, this makes the build more fragile.

SimeonAleksov and others added 2 commits December 2, 2022 15:13
@knyghty
Copy link
Member

knyghty commented Dec 4, 2022

Thanks for having a look at this! I'll try to review in the coming days.

@knyghty knyghty self-requested a review December 4, 2022 14:25
Copy link
Member

@knyghty knyghty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. I left some comments. I am not too sure about the mypy errors relating to the plugins. For guesslang not being found you can add guesslang.* to the excludes: https://mypy.readthedocs.io/en/stable/config_file.html#confval-exclude

@@ -0,0 +1,23 @@
from typing import Optional
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't much like the name of this file, can it be utils? Or whatever is more appropriate.

Comment on lines +90 to +92
def get_parser(
message: hikari.Message,
) -> Optional[Union[HTMLParser, PythonParser, JavascriptParser]]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're on 3.10 (which reminds me, should probably upgrade to 3.11), so we can do:

Suggested change
def get_parser(
message: hikari.Message,
) -> Optional[Union[HTMLParser, PythonParser, JavascriptParser]]:
def get_parser(
message: hikari.Message,
) -> HTMLParser | PythonParser | JavascriptParser || None:

Additionally... Couldn't this be:

Suggested change
def get_parser(
message: hikari.Message,
) -> Optional[Union[HTMLParser, PythonParser, JavascriptParser]]:
def get_parser(message: hikari.Message) -> CodeblockParser | None:

NamedTuple(
"MarkdownCodeblock",
[
("content", Optional[str]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
("content", Optional[str]),
("content", str | None),

return f"`{text}`"


def codeblock(text: str, language: Language) -> Optional[str]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def codeblock(text: str, language: Language) -> Optional[str]:
def codeblock(text: str, language: Language) -> str | None:

@knyghty
Copy link
Member

knyghty commented Dec 5, 2022

I guess for the plugin stuff in mypy, maybe just typing.cast()ing them to callables will work?

@tim-schilling
Copy link
Author

It's probably unlikely I'm going to get back around to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants