-
Notifications
You must be signed in to change notification settings - Fork 295
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
🖊️ Preserve localization information for numbers and booleans #5676
Conversation
7e75873
to
7cfe631
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the change, and that you addressed so many issues with it.
I tried every example and all of them worked! I think this is ready to ship it :D
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
Fixes #5548
Goal
The goal of this PR is make Hedy understand and preserve localization information when using numbers and boolean values. In other words, if the user types
1 + 1
the output should be2
and if the input is١ + ١
the output should be٢
. Similarly, if the user typesa = True\nprint a
the output should beTrue
and if the input isa = Вярно
, the output should beВярно
. The numeral system or the textual representation of the boolean values should not be based on a static setting somewhere in the user profile. The localization information of the input and output should be dynamic.Brief implementation notes
To make Hedy understand what numeral system or boolean values the user is using, I had to change the basic types of the transpiled code. So before, when the user executes a = ١, we would transpile it to a = 1. Now, the variable needs to be assigned to a type that not only holds the real value of 1 but also the localisation information, e.g. that the Arabic numeral system is used. The value is now transpiled to a = Value(1, num_sys=‘Arabic’). Similarly, for booleans we store information about the textual representation of the value: b = waar becomes b = Value(True, bools={True: ‘waar’, False: ‘vals’}).
Later, when the variable is referenced, we decide whether to use the real value or the presentation value based on the command. For example, if the variable is used in + operator, we definitely need the real value (1 and True). However, if the variable is used in a print or ask command, we need its presentation (١ and waar).
Let me give you a few more examples, so that you get a better understanding of the changes:
Issues addressed as a side-effect
In order to make this change possible, a few issues had to be addressed on the fly:
sum
instead of 2. However, if we change the name of the variable tosun
, it outputs 2.How to test
Few of the unit tests can verify that the desired output is produced and, also, there are substantial differences between Python and Skulpt. Thus, the only way to ensure that the change does not lead to regression is to execute the main scenarios of all commands in the browser and manually validate their outcome.
Please check that no other functionality is broken, e.g. variable list appears correctly, debugging works as expected, etc.