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

Pymdown blocks support #1316

Merged
merged 13 commits into from Nov 1, 2023
Merged

Conversation

orcephrye
Copy link
Contributor

@orcephrye orcephrye commented Oct 20, 2023

This is for Issue: #1197

I need help!

There are several things that are not complete about this PR. However, it is working and it is a start and I figured I could use this as a way to get help.

  1. I am not a CSS person so there are questions around how to get these different blocks to look correct. Particularly with detail/summary tags. I also wanted to use font awesome icons but I couldn't get that to work for some reason.

  2. Currently, the sidebar for the markdown editor only supports macros and seems to exist only within the macros plugin. But it makes sense that PyMdown extensions should show up in that sidebar right? However, I am unsure as to how to proceed with that.

There may be more things to discuss but this is a starting place.

@orcephrye
Copy link
Contributor Author

Also I don't know why these tests are failing. Running >hatch run test:all had all passing. I will take another look. But these tests have been problematic as of late.

@codecov
Copy link

codecov bot commented Oct 21, 2023

Codecov Report

Merging #1316 (3aa6a9f) into main (464fb05) will decrease coverage by 0.04%.
The diff coverage is 72.72%.

@@            Coverage Diff             @@
##             main    #1316      +/-   ##
==========================================
- Coverage   79.39%   79.36%   -0.04%     
==========================================
  Files         109      113       +4     
  Lines        4703     4725      +22     
  Branches      635      635              
==========================================
+ Hits         3734     3750      +16     
- Misses        759      765       +6     
  Partials      210      210              
Files Coverage Δ
src/wiki/plugins/pymdown/__init__.py 100.00% <100.00%> (ø)
src/wiki/plugins/pymdown/settings.py 100.00% <100.00%> (ø)
src/wiki/plugins/pymdown/wiki_plugin.py 100.00% <100.00%> (ø)
src/wiki/plugins/pymdown/apps.py 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@orcephrye
Copy link
Contributor Author

Also I don't know why these tests are failing. Running >hatch run test:all had all passing. I will take another look. But these tests have been problematic as of late.

I fixed it. It was a caused bunch some merges that screwed up the pyproject.toml file. The version I had no longer had the pymdown-extension dependency but the package was still installed in my test environments. So it was working for me but failing here.

@oscarmcm
Copy link
Member

Thanks for the PR @orcephrye I'll take a look on this.

Im just worried by the codecov decrease.

@oscarmcm oscarmcm self-assigned this Oct 23, 2023
@oscarmcm oscarmcm added enhancement A feature request: Will be implemented if someone steps up! plugins Feature or bug related to plugin API labels Oct 23, 2023
@orcephrye
Copy link
Contributor Author

Thanks for the PR @orcephrye I'll take a look on this.

Im just worried by the codecov decrease.

I am extremely confused by this and thus a little worried too. So the 72.72% is from the diff not the whole project. It looks... sorta like it is complaining that I am not testing the apps.py file? But the apps.py is just a simple little configuration. I can add a 'test' for it but it feels pointless and stupid.

@oscarmcm
Copy link
Member

yeah, I think the same too.

Let me see what I can do, maybe tell codecov to not read that file will help us

@oscarmcm oscarmcm merged commit d416db3 into django-wiki:main Nov 1, 2023
13 of 15 checks passed
@oscarmcm
Copy link
Member

oscarmcm commented Nov 1, 2023

Thanks for the help with this @orcephrye 🎉

@orcephrye
Copy link
Contributor Author

Thanks for the help with this @orcephrye 🎉

Hey... I felt like this PR wasn't finished. Sorry for any confusion. Perhaps I should have just linked to a branch and kept talking about Blocks in the Issue instead. But I do not feel that this is a finished solution. It works. If the plugins are enabled then the PyMarkdown blocks (minus Tab) will work.

However, I have questions about the styling/CSS for the blocks. IE: Does the color fit? Should Icons for the different types be a good idea? How can I use the icons that are already setup in scss? I tried and couldn't figure out how to use them.

Also, I think we should add documentation to the Marco sidebar so that someone who has this plugin enabled can see how to take advantage of it. The problem is that currently that is located within the marcos plugin. Not sure the best way to go about adding to that.

@benjaoming
Copy link
Member

Good that you mention it @orcephrye - I had the impression that it wasn't finished because of your comment in the PR description, and I originally wanted to have tested it out manually to understand layout issues.

But it's always nice to get things merged :) You can use the "draft" state for the PRs to ensure that there's full clarity about the state of the work. I sometimes also add "WIP" to the PR title to make sure it's really really clear 😂

@benjaoming
Copy link
Member

But now harm done here, seems the additional improvements can come in follow-up PRs 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature request: Will be implemented if someone steps up! plugins Feature or bug related to plugin API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants