Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring: split into files #452

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

Einenlum
Copy link

Hi there.

First, thank you for your great work ❤️

Following this discussion #382 , I wanted to try to make the extra part more flexible.
I realized it's quite hard to follow the code because everything is packed in a huge file.

This PR is a first step to refactor the code and to then make the extras more flexible/pluggable.

Wolud you agree to split this code? I don't mind about the name of the files if you don't like this organization. But I think it would be nice to avoid having just one big pile of code.

@Einenlum Einenlum force-pushed the refactor/split-into-files branch from 7f58bc2 to adb5b21 Compare June 16, 2022 22:47
@nicholasserra
Copy link
Collaborator

@Crozzers Youve been in this codebase more than I have recently. Curious if you have any opinions on this refactor and the linked issue?

@Crozzers
Copy link
Contributor

Crozzers commented Jun 20, 2022 via email

@Crozzers
Copy link
Contributor

Crozzers commented Jun 20, 2022 via email

@Einenlum Einenlum force-pushed the refactor/split-into-files branch from f5c6d92 to 2a56b90 Compare June 21, 2022 07:44
@Einenlum
Copy link
Author

out of curiosity, what is the blank parser_objects.py file for?

Woops, that was a mistake. I just pushed force the git history to remove that blank file. Thank you! :)

Oh and in terms of the linked issue, I think custom extras would be a great feature to have but it might get complicated for the end user when configuring when their custom extra is triggered. They might, for example, want to process custom link syntax before _do_links is triggered, or they might wish to post-process them.

Seems like Python Markdown uses Preprocessors and Postprocessors. Maybe it's the way?

@nicholasserra
Copy link
Collaborator

We have pre and post hooks. My worry is that there's simply no way to really refactor-out the extras stuff from the core library without rewriting the whole thing.

Maybe keep exploring your ideas @Einenlum assuming this was merged and see where you land. I think this is probably fine to land either way but i'm kind of curious to see what's even possible after this for the extras stuff.

@Tobi-De
Copy link

Tobi-De commented Sep 25, 2022

Hey @Einenlum, hope you are doing well.
I was trying to add a new extension to the package to display mermaid diagrams and realized that the code was too complicated to extend and found this pull request. Since there is no recent activity on this, I was wondering how things were going. I am available to help if needed.

@Einenlum
Copy link
Author

Hey @Tobi-De,
Sorry for the very late reply. Honestly I ended up using https://python-markdown.github.io/ and its extensions, which did what I expected.

I don't have any incentive to work on this now, but I wish good luck to anyone who wants to dive into it and refactor things :)

@Tobi-De
Copy link

Tobi-De commented Nov 14, 2022

Hey @Tobi-De, Sorry for the very late reply. Honestly I ended up using https://python-markdown.github.io/ and its extensions, which did what I expected.

I don't have any incentive to work on this now, but I wish good luck to anyone who wants to dive into it and refactor things :)

I understand, that's fair.
Personally, I found mistune very interesting, I haven't tried it yet but I will probably use it for what I am working on.

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.

4 participants