Test case for regression involving single element placeables #108
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
So, the use-isolation one I was expecting in a different scenario, actually. When we make the transform work on the Transformer api, it will modify the AST in place, and that would be bundle-dependent. I guess that adding and discarding |
I think that should do it for the other issue - #109 - did this comment get mixed up? |
Pike
added a commit
to Pike/python-fluent
that referenced
this pull request
Apr 3, 2019
This patch removes state from the compiled tree. For one, the recursion detection on patterns keeps its state in the `ResolverEnvironment` now, instead of in the compiled tree. Also, the question of bidi isolation is now kept in the resolver context, instead of in the compiled tree. For single placeables, which don't get bidi-isolated ever, there's now a distinct NeverIsolatingPlaceable, which also takes care of resolving fluent values to strings. This should fix the test cases in projectfluent#108 and projectfluent#109
|
Thanks for the test cases. I've included them in #115, as well as the fix. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This looks like a regression @Pike (I have another one coming too, sorry to do that to you on a Friday afternoon...!). Numbers and
FluentNoneare not getting turned into strings as they should.I realized I didn't understand your new implementation as well as I need to, especially in terms of at what point things get resolved, so I was trying to add some specific tests and came across some issues.
Only 3 of these tests fail (those with
use_isolating=True), the others are there for good measure.I think you probably have a better idea how to fix these at this point in time, shall I leave them with you?