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

Order of filter arguments 'attribute' and 'case_sensitive' #736

Closed
davidism opened this issue Jul 6, 2017 · 6 comments
Closed

Order of filter arguments 'attribute' and 'case_sensitive' #736

davidism opened this issue Jul 6, 2017 · 6 comments

Comments

@davidism
Copy link
Member

davidism commented Jul 6, 2017

sort initially didn't take an attribute argument, so when it was added, case_sensitive came first. Therefore, you have to do sort(attribute='name') to specify the attribute only. Now we've added unique, min, and max, all of which take both arguments. I think it makes more sense for attribute to come first, but for consistency should we keep it second even for new filters?

Aside: I also think case_sensitive should default to true, not false, especially since str.lower is not very robust for Unicode.

@ThiefMaster
Copy link
Member

Aside: I also think case_sensitive should default to true

Not for sorting for sure. In templates you almost never want a case-sensitive sort, it's not very human-friendly and that's what sorting in templates is usually used for.

@davidism
Copy link
Member Author

davidism commented Jul 6, 2017

#737 makes the order attribute, case_sensitive for unique, min, and max, but we can change it back if we decide that's what we want.

@ThiefMaster
Copy link
Member

While I think people should be using a kwarg for case_sensitive since |sort(true) is not very clear, we can't expect everyone to have done that. So I'd prefer to not break people's templates...

@davidism
Copy link
Member Author

davidism commented Jul 6, 2017

Not going to change sort or dictsort, just talking about new filters. Mostly concerned with consistency: if sort is one way and unique is the other, people may be confused.

@ThiefMaster
Copy link
Member

I'd rather keep the same order in the new filters to avoid this kind of confusion. I think using kwargs for any of these arguments is clearer anyway

@davidism
Copy link
Member Author

davidism commented Jul 6, 2017

Yeah, being consistent is probably a better idea here. I'll change the examples to use kwargs consistently too.

@davidism davidism closed this as completed Jul 6, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants