Log warnings about files that would have been processed by disabled readers

[boxydog]

Resolves: 1868

• Ensured tests pass and (if applicable) updated functional test output
• Conformed to code style guidelines by running appropriate linting tools
• Added tests for changed code
• Updated documentation for changed code

This is a proof of concept of producing legible warning messages if markdown is not installed.

Example output:

WARNING ./sample.md: Could not import markdown.Markdown. Have you installed the markdown package? readers.py:688
Done: Processed 0 articles, 0 drafts, 0 hidden articles, 0 pages, 0 hidden pages and 0 draft pages in 0.07 seconds.

If this PR looks good, I can try to write at least one unit test of the new functionality.

Some downsides:

1. this is not super simple
2. it will produce one warning per file

I don't think 2 is a big deal. I think 1 is a judgment call, I'm interested in opinions.

It would probably be easier to just require the markdown package?

[boxydog]

Looking at the failing tests..

[justinmayer]

Nice work, @boxydog! I made a couple of minor comments.

[justinmayer]

Looks good to me! 👍 Any further comments from @getpelican/reviewers before merging?

[justinmayer]

@boxydog said:

If this PR looks good, I can try to write at least one unit test of the new functionality.

Do you think it would be worthwhile to add additional unit test coverage to this PR before it is merged?

[boxydog]

@boxydog said:

If this PR looks good, I can try to write at least one unit test of the new functionality.

Do you think it would be worthwhile to add additional unit test coverage to this PR before it is merged?

Yes, I think that's a good idea, otherwise I'll forget. I'll work on it.

Fyi, my personal philosophy is "epsilon testing", my term for, "write some tests, more than zero", meaning I don't aim for 100% coverage (as that can feel like more work than it's worth to me).

[boxydog]

Coverage on master:

TOTAL                                  3918    984    75%

Required test coverage of 74% reached. Total coverage: 74.89%

Coverage on this branch with added tests:

TOTAL                                  3950    982    75%

Required test coverage of 74% reached. Total coverage: 75.14%

I'm happy for you to merge this if you like it.

However, FYI, if I were emperor of pelican, I'd just require the markdown library. Less code, simpler to explain. We want to support markdown anyway, so we'll want to support that "optional" dependency.

[boxydog]

Ugh, tests failed! I'll look..

[boxydog]

Ugh, tests will be more complicated than I had hoped. I changed Markdown.enabled to False, but presumably if some other test is running at the same time it gets disturbed. So probably I have to mock a Markdown class that has a different enabled.

[boxydog]

However, FYI, if I were emperor of pelican, I'd just require the markdown library. Less code, simpler to explain. We want to support markdown anyway, so we'll want to support that "optional" dependency.

.. and then I'd close this PR instead of merge it.

Again, whatever you want, I just want to move important things forward.

[justinmayer]

Given the continued interest in alternative Markdown parsers (e.g., #3344), there may be value in leaving Python-Markdown as an optional dependency. I am therefore inclined to merge this.

Many thanks for this enhancement, @boxydog! 🏅