pallets / jinja

A very fast and expressive template engine.
https://jinja.palletsprojects.com
BSD 3-Clause "New" or "Revised" License
10.23k stars 1.6k forks source link

[Docs] Fix typo on filter name #1911

Open vbuxbaum opened 9 months ago

vbuxbaum commented 9 months ago

Hi everyone!

I was studying Jinja and Flask with @JefersonViana , and noticed a typo in the docs about custom filters.

I hope there is no need for creating an issue for this.

Let me know if you think that there is a better solution.

davidism commented 9 months ago

The typo would be in the use, not the registration. Always prefer separated words, so |datetime_filter. (Yes, "datetime" itself doesn't follow this, but it's the built-in name.)

davidism commented 9 months ago

Be sure to make docs fixes against the latest feature branch (3.1.x) rather than main so that it gets into the current docs. You'll need to rebase and force push to fix that:

$ git rebase --onto origin/3.1.x origin/main
$ git push -f

After you do that you'll only see your commit on this page instead of the huge list.

vbuxbaum commented 9 months ago

Be sure to make docs fixes against the latest feature branch (3.1.x)

Thanks @davidism ! I did not pay attention to that, sorry. Done ✅

The typo would be in the use, not the registration. Always prefer separated words, so |datetime_filter. (Yes, "datetime" itself doesn't follow this, but it's the built-in name.)

The current filter's name is datetime_format. Your suggestion is to change to datetime_filter, or it was a typo? I don't think using _filter in a filter name is necessary, although it helps the explanation.

I also prefer naming separating words with _, but I think it is better if the filter function and the register in environment.filters have different names. Someone (mainly beginners) may wonder

What if we rename the datetime_format function to datetime_format_filter, and keep the filter's name as datetime_format?

davidism commented 9 months ago

datetime_format everywhere please.

vbuxbaum commented 9 months ago

datetime_format everywhere please.

@davidism done ✅

vbuxbaum commented 9 months ago

The pre-commit.ci failure has nothing to do with my changes, right? Do I need to do something about it?