search

pelican-plugins / pandoc-reader

Pandoc Reader is a Pelican plugin that processes Markdown content via Pandoc
11 stars 3 forks source link

Add tests for Pandoc reader #5

Closed nandac closed 3 years ago

nandac commented 3 years ago

Final commit for now.

I am not happy with the way isort sorted the imports for text_pandoc_reader.py as it put a local import pandoc_reader above the pelican imports. Kindly let me know if I should tweak the isort settings to get this to work correctly.

I was wondering if it is okay to change the markdown heading style in the README to use # and ## instead of ===== or ----- style since I am using multi-level headings and want to be consistent.

justinmayer commented 3 years ago

Regarding the import sorting, I agree that it needs adjusting. I suggest the following:

# Designate "pelican" and "pandoc_reader" as separate import sections
known_pelican = "pelican"
known_pandoc = "pandoc_reader"

sections = "FUTURE,STDLIB,THIRDPARTY,PELICAN,PANDOC,FIRSTPARTY,LOCALFOLDER"

Regarding the Markdown heading style, I spend a lot of time looking at plain-text Markdown (i.e., not rendered), and I usually find the underlined headers much more "scannable" and readable than # / ##. Personally, I don't think there's anything wrong with mixing the two styles, using underlines for major headings 1 & 2 and then ### / #### for minor headings. But I don't feel particularly strongly about this, so you are free to change them if it's important to you.

nandac commented 3 years ago

Thanks @justinmayer. I have separated out the tests which definitely makes them a lot more manageable.

I have also fixed the import order with your suggestions and it works well.

I am not too worried about heading styles in Markdown myself. It is just that my vscode Markdown linter complains about mixing the two styles which I can switch off on my end.

I think the only thing left is figuring out under what name we want to publish the plugin. The existing Pelican-Pandoc-Reader in pyPI is significantly different from mine with respect to implementation and features.

justinmayer commented 3 years ago

Everything here looks good to me. Well done! 🥇

Regarding the package name, I already opened an issue in this repo in hopes of merging our efforts and utilizing the existing package name for the next release. Let's allow for some time for a potential response then decide what to do next.