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

[Docs] Fix typo on filter name #1911

Open
wants to merge 2 commits into
base: stable
Choose a base branch
from
Open

Conversation

vbuxbaum
Copy link

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
Copy link
Member

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 davidism added the docs label Nov 17, 2023
@davidism
Copy link
Member

davidism commented Nov 17, 2023

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.

@davidism davidism changed the base branch from main to 3.1.x November 17, 2023 02:07
@vbuxbaum
Copy link
Author

vbuxbaum commented Nov 17, 2023

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

  • "is it necessary to keep the same name?"
  • or even "which should be used inside the template? the function's name or the string registered in environment.filters"

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

@davidism
Copy link
Member

datetime_format everywhere please.

@vbuxbaum
Copy link
Author

datetime_format everywhere please.

@davidism done ✅

@vbuxbaum
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants