8000 Fixed #32208 -- fixed misinterpretation of "__proxy__" as TypeError. by Blackeyeforreal · Pull Request #13707 · django/django · GitHub
[go: up one dir, main page]
Skip to content

Fixed #32208 -- fixed misinterpretation of "__proxy__" as TypeError. #13707

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

Jump to bottom
Closed
wants to merge 7 commits into from
Closed

Fixed #32208 -- fixed misinterpretation of "__proxy__" as TypeError. #13707

wants to merge 7 commits into from

Conversation

Blackeyeforreal
Copy link

tweaked exception handling of add built in filter , to support (gettext
_lazy). as previously it was taking it as a TypeError and used to
return a string.

@Blackeyeforreal
Fixed #32208 -- fixed misinterpretation of "__proxy__" as TypeError .
6c661a7 
tweaked exception handling of add built in filter , to support (gettext
_lazy). as previously it was taking it as a TypeError and used to
return a string.
@Blackeyeforreal
Fixed #32208 -- fixed misinterpretation of "__proxy__" as TypeError.
06e9617 
modified try and except web of add built in filter to support gettext_lazy()
as previously it was only sending an empty string on passing gettext_lazy
as an argument.
@Blackeyeforreal
Fixed #32208 -- fixed misinterpretation of "__proxy__" as TypeError .
726ecdc 
tweaked exception handling of add built in filter , to support (gettext
_lazy). as previously it was taking it as a TypeError and used to
return a string.
@Blackeyeforreal
Fixed #32208 -- fixed misinterpretation of "__proxy__" as TypeError .
f4b1e06 
tweaked exception handling of add built in filter , to support (gettext
_lazy). as previously it was taking it as a TypeError and used to
return a string.
@Blackeyeforreal
Fixed #32208 -- fixed misinterpretation of "__proxy__" as TypeError .
827851e
@Blackeyeforreal
Fixed #32208 -- fixed misinterpretation of "__proxy__" as TypeError.
ffe204a 
tweaked exception handling of add built in filter , to support (gettext
_lazy). as previously it was taking it as a TypeError and used to
return a string.
@Blackeyeforreal
Fixed #32208 -- fixed misinterpretation of "__proxy__" as TypeError.
4bc5e52 
tweaked exception handling of add built in filter , to support (gettext
_lazy). as previously it was taking it as a TypeError and used to
return a string.
@felixxm
Copy link
Member
felixxm commented Nov 22, 2020

@Blackeyeforreal You don't need to open a new PR for each change. You can push commits to an opened PR.

@Blackeyeforreal
Copy link
Author
8000 Blackeyeforreal commented Nov 22, 2020

@Blackeyeforreal You don't need to open a new PR for each change. You can push commits to an opened PR.
thanks, i was wondering how it can so inconvenient.

@claudep
Copy link
Member
claudep commented Nov 22, 2020

I had thought about another approach where we would add str() of each value in case of exceptions. However this would be slightly backwards incompatible because previously 5|add:'string' would have returned an empty string while the new approach would produce 5string. However counting on type errors to produce an empty result doesn't look very solid, so I'd be tempted to go and document the backwards incompatibility. @felixxm, thoughts?

Then I also think we should explore this:

diff --git a/django/utils/functional.py b/django/utils/functional.py
index 6d38f932f9..5c8a0c233f 100644
--- a/django/utils/functional.py
+++ b/django/utils/functional.py
@@ -176,6 +176,12 @@ def lazy(func, *resultclasses):
                 return str(self) % rhs
             return self.__cast() % rhs
 
+        def __add__(self, other):
+            return self.__cast() + other
+
+        def __radd__(self, other):
+            return other + self.__cast()
+
         def __deepcopy__(self, memo):
             # Instances of this class are effectively immutable. It's just a
             # collection of functions. So we don't need to do anything
diff --git a/tests/template_tests/filter_tests/test_add.py b/tests/template_tests/filter_tests/test_add.py
index 0fcc661f4a..dc765b37ee 100644
--- a/tests/template_tests/filter_tests/test_add.py
+++ b/tests/template_tests/filter_tests/test_add.py
@@ -46,6 +46,21 @@ class AddTests(SimpleTestCase):
         output = self.engine.render_to_string('add07', {'d': date(2000, 1, 1), 't': timedelta(10)})
         self.assertEqual(output, 'Jan. 11, 2000')
 
+    @setup({'add08': '{{ s1|add:lazy_s2 }}'})
+    def test_add08(self):
+        from django.utils.translation import gettext_lazy
+        output = self.engine.render_to_string('add08', {'s1': 'string', 'lazy_s2': gettext_lazy('lazy')})
+        self.assertEqual(output, 'stringlazy')
+
+    @setup({'add09': '{{ lazy_s1|add:lazy_s2 }}'})
+    def test_add09(self):
+        from django.utils.translation import gettext_lazy
+        output = self.engine.render_to_string(
+            'add09',
+            {'lazy_s1': gettext_lazy('string'), 'lazy_s2': gettext_lazy('lazy')}
+        )
+        self.assertEqual(output, 'stringlazy')
+
 
 class FunctionTests(SimpleTestCase):

@felixxm
Copy link
Member
felixxm commented Nov 23, 2020

I had thought about another approach where we would add str() of each value in case of exceptions. However this would be slightly backwards incompatible because previously 5|add:'string' would have returned an empty string while the new approach would produce 5string. However counting on type errors to produce an empty result doesn't look very solid, so I'd be tempted to go and document the backwards incompatibility. @felixxm, thoughts?

I'm not sure about this approach, casting to str can create an unexpected results, e.g. 'string'|add:None will return stringNone.

Then I also think we should explore this:
...

This looks nice.

@Blackeyeforreal
Copy link
Author

I had thought about another approach where we would add str() of each value in case of exceptions. However this would be slightly backwards incompatible because previously 5|add:'string' would have returned an empty string while the new approach would produce 5string. However counting on type errors to produce an empty result doesn't look very solid, so I'd be tempted to go and document the backwards incompatibility. @felixxm, thoughts?

Then I also think we should explore this:

diff --git a/django/utils/functional.py b/django/utils/functional.py
index 6d38f932f9..5c8a0c233f 100644
--- a/django/utils/functional.py
+++ b/django/utils/functional.py
@@ -176,6 +176,12 @@ def lazy(func, *resultclasses):
                 return str(self) % rhs
             return self.__cast() % rhs
 
+        def __add__(self, other):
+            return self.__cast() + other
+
+        def __radd__(self, other):
+            return other + self.__cast()
+
         def __deepcopy__(self, memo):
             # Instances of this class are effectively immutable. It's just a
             # collection of functions. So we don't need to do anything
diff --git a/tests/template_tests/filter_tests/test_add.py b/tests/template_tests/filter_tests/test_add.py
index 0fcc661f4a..dc765b37ee 100644
--- a/tests/template_tests/filter_tests/test_add.py
+++ b/tests/template_tests/filter_tests/test_add.py
@@ -46,6 +46,21 @@ class AddTests(SimpleTestCase):
         output = self.engine.render_to_string('add07', {'d': date(2000, 1, 1), 't': timedelta(10)})
         self.assertEqual(output, 'Jan. 11, 2000')
 
+    @setup({'add08': '{{ s1|add:lazy_s2 }}'})
+    def test_add08(self):
+        from django.utils.translation import gettext_lazy
+        output = self.engine.render_to_string('add08', {'s1': 'string', 'lazy_s2': gettext_lazy('lazy')})
+        self.assertEqual(output, 'stringlazy')
+
+    @setup({'add09': '{{ lazy_s1|add:lazy_s2 }}'})
+    def test_add09(self):
+        from django.utils.translation import gettext_lazy
+        output = self.engine.render_to_string(
+            'add09',
+            {'lazy_s1': gettext_lazy('string'), 'lazy_s2': gettext_lazy('lazy')}
+        )
+        self.assertEqual(output, 'stringlazy')
+
 
 class FunctionTests(SimpleTestCase):

well code seams to be handling the new test case (ie add08 and add 09) fine, although i think it would be nice if we add it, as a precaution. But i know it is gonna sound silly but how do i push changes in this pull request.
also how do i get this reviewed , should i wait for someone to look at it , or should i tag someone .

@jacobtylerwalls
Copy link
Member

Hi there, you can push new commits to the branch you used to open the pull request, in your case, ticket_99999.

git push origin ticket_99999

Does that help?

For reviews, you don't need to tag anyone. The most you need to do is keep an eye on the ticket flags and make sure if anyone sets "Needs ..." that you uncheck it when you're ready for a review.

@smithdc1
Copy link
Member

Superseded by #13794

@smithdc1 smithdc1 closed this Dec 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Blackeyeforreal @felixxm @claudep @jacobtylerwalls @smithdc1
0