Skip to content

Feature/prompt user to paste code properly 35 - Take 2#137

Open
tim-schilling wants to merge 2 commits into
django-discord:mainfrom
tim-schilling:feature/prompt-user-to-paste-code-properly-35
Open

Feature/prompt user to paste code properly 35 - Take 2#137
tim-schilling wants to merge 2 commits into
django-discord:mainfrom
tim-schilling:feature/prompt-user-to-paste-code-properly-35

Conversation

@tim-schilling

Copy link
Copy Markdown

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

knyghty commented Dec 4, 2022

Copy link
Copy Markdown
Member

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

@knyghty knyghty left a comment

Copy link
Copy Markdown
Member

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

Comment thread bot/markup/services.py
@@ -0,0 +1,23 @@
from typing import Optional

Copy link
Copy Markdown
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 thread bot/codeblocks/parsers.py
Comment on lines +90 to +92
def get_parser(
message: hikari.Message,
) -> Optional[Union[HTMLParser, PythonParser, JavascriptParser]]:

Copy link
Copy Markdown
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:

Comment thread bot/codeblocks/types.py
NamedTuple(
"MarkdownCodeblock",
[
("content", Optional[str]),

Copy link
Copy Markdown
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),

Comment thread bot/markup/services.py
return f"`{text}`"


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

Copy link
Copy Markdown
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

knyghty commented Dec 5, 2022

Copy link
Copy Markdown
Member

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

@tim-schilling

Copy link
Copy Markdown
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