Skip to content

common : honor case_sensitive argument in jinja sort and dictsort #24971

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.

Sign up for GitHub

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

Closed
dnislno wants to merge 1 commit into from
Closed

common : honor case_sensitive argument in jinja sort and dictsort #24971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions common/jinja/value.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1077,8 +1077,7 @@ const func_builtins & value_array_t::get_builtins() const {
value val_reverse = args.get_kwarg_or_pos("reverse", 1);
value val_case = args.get_kwarg_or_pos("case_sensitive", 2);
value attribute = args.get_kwarg_or_pos("attribute", 3);
// FIXME: sorting is currently always case sensitive
//const bool case_sensitive = val_case->as_bool(); // undefined == false
const bool case_sensitive = val_case->as_bool(); // undefined == false
const bool reverse = val_reverse->as_bool(); // undefined == false
const bool attr_is_int = is_val<value_int>(attribute);
const int64_t attr_int = attr_is_int ? attribute->as_int() : 0;
Expand All @@ -1097,6 +1096,11 @@ const func_builtins & value_array_t::get_builtins() const {
throw raised_exception("sort: unsupported object attribute comparison between " + a->type() + " and " + b->type());
}
}
if (!case_sensitive && is_val<value_string>(val_a) && is_val<value_string>(val_b)) {
const std::string sa = val_a->as_string().lowercase().str();
const std::string sb = val_b->as_string().lowercase().str();
return reverse ? (sa > sb) : (sa < sb);
}
return value_compare(val_a, val_b, reverse ? value_compare_op::gt : value_compare_op::lt);
});
return is_val<value_tuple>(val) ? mk_val<value_tuple>(std::move(arr)) : mk_val<value_array>(std::move(arr));
Expand Down Expand Up @@ -1194,17 +1198,19 @@ const func_builtins & value_object_t::get_builtins() const {
value val_case = args.get_kwarg_or_pos("case_sensitive", 1);
value val_by = args.get_kwarg_or_pos("by", 2);
value val_reverse = args.get_kwarg_or_pos("reverse", 3);
// FIXME: sorting is currently always case sensitive
//const bool case_sensitive = val_case->as_bool(); // undefined == false
const bool case_sensitive = val_case->as_bool(); // undefined == false
const bool reverse = val_reverse->as_bool(); // undefined == false
const bool by_value = is_val<value_string>(val_by) && val_by->as_string().str() == "value" ? true : false;
auto result = mk_val<value_object>(val_input); // copy
std::sort(result->val_obj.begin(), result->val_obj.end(), [&](const auto & a, const auto & b) {
if (by_value) {
return value_compare(a.second, b.second, reverse ? value_compare_op::gt : value_compare_op::lt);
} else {
return value_compare(a.first, b.first, reverse ? value_compare_op::gt : value_compare_op::lt);
value val_a = by_value ? a.second : a.first;
value val_b = by_value ? b.second : b.first;
if (!case_sensitive && is_val<value_string>(val_a) && is_val<value_string>(val_b)) {
const std::string sa = val_a->as_string().lowercase().str();
const std::string sb = val_b->as_string().lowercase().str();
return reverse ? (sa > sb) : (sa < sb);
}
return value_compare(val_a, val_b, reverse ? value_compare_op::gt : value_compare_op::lt);
});
return result;
}},
Expand Down
18 changes: 18 additions & 0 deletions tests/test-jinja.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,18 @@ static void test_filters(testing & t) {
"xyz"
);

test_template(t, "sort case_sensitive=false",
"{{ items|sort(case_sensitive=false)|join(',') }}",
{{"items", json::array({"Banana", "apple", "Cherry", "date"})}},
"apple,Banana,Cherry,date"
);

test_template(t, "sort case_sensitive=true",
"{{ items|sort(case_sensitive=true)|join(',') }}",
{{"items", json::array({"Banana", "apple", "Cherry", "date"})}},
"Banana,Cherry,apple,date"
);

test_template(t, "join",
"{{ items|join(', ') }}",
{{"items", json::array({"a", "b", "c"})}},
Expand Down Expand Up @@ -1755,6 +1767,12 @@ static void test_object_methods(testing & t) {
"A=1 B=2 a=1 b=2 c=3 "
);

test_template(t, "dictsort case insensitive",
"{% for k, v in obj|dictsort(case_sensitive=false) %}{{ k }}={{ v }} {% endfor %}",
{{"obj", {{"Banana", 1}, {"apple", 2}, {"Cherry", 3}}}},
"apple=2 Banana=1 Cherry=3 "
);

test_template(t, "object|tojson",
"{{ obj|tojson }}",
{{"obj", {{"name", "test"}, {"value", 42}}}},
Expand Down