pelican
pelican copied to clipboard
getpelican
Jinja2's autoescape compatibility for truncate_html_words()
Added support for Jinja2 generated end marker when the passed variable is an instance of Markup. This makes the function compatible with Jinja2's environments where autoescape is enabled.
Pull Request Checklist
Resolves: #2917
- [ ] Ensured tests pass and (if applicable) updated functional test output
- [x] Conformed to code style guidelines by running appropriate linting tools
- [x] Added tests for changed code
- [ ] Updated documentation for changed code
I am looking at the failed tests and I am puzzled re: how my change could have affected these three tests. Moreover, when I run tox on a clean clone of the repository (without my changes in), I am also getting these three test failed. At the same time, I see that the latest commit in Jul passed all the tests. Could somebody help me understand what's going on, please?
There are two CI failures. The first one is indeed unrelated to your changes and is instead caused by a newer version of FeedGenerator, for which we have not yet updated our functional test output.
The second one is due to code style conformance checks. Did you follow the development docs and run invoke lint before committing changes? If you make sure you have Pre-commit installed, it should catch those problems even if you forget to run invoke lint.
… and the more I look at the diff caused by the latest version of FeedGenerator, the more I wonder whether FeedGenerator 1.9.2 has changed feed output in an undesirable manner. Issue filed: https://github.com/getpelican/feedgenerator/issues/30 🤔
Yeah, I also found that FeedGenerator has been updated on Aug 18 and the diff I made shows that the difference is actually the missing <subtitle></subtitle> -- we are passing None as the value, so most likely it was optimised out.
@justinmayer , I did fix linting issues (there is a commit just before your comment). I was relying on my Vim being good with flake8 rules, but it was not, so when I've seen the linting errors, I quickly amended the file :).
@avaris correctly pointed out that I am tapping into an internal function (truncate_html_words()), which is not exposed to Jinja2 as a filter by default. I agree with that assessment, so I propose to close the corresponding issue (#2917 ). However, I would love to preserve the additional tests introduced in this PR. Will it be OK if I drop the change to truncate_html_words() and the test for it, then rename the PR to be "Adding missing tests for truncate_html_words()" instead? The tests are covering a situation when a custom end_text was provided, e.g. through the SUMMARY_END_SUFFIX parameter in pelicanconf.py.
@galaxy4public: That seems to me a reasonable path forward on this topic. Would you please proceed as you suggested above?
@justinmayer , will do that (most likely this coming weekend since I am currently smashed with work :( )
Hi @galaxy4public. Just checking in regarding your pull request. Do you think you could find some time to move it forward so we can get it merged?
@justinmayer, sorry, I was overloaded with other work lately. Should be able to work on this by the end of this week.