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

Icons for "Move left, Move right, Remove character", replaced by unicode strings #65

Open
nswarup14 opened this issue Jun 30, 2019 · 14 comments

Comments

@nswarup14
Copy link

Attached screenshot below which shows the problem
calculate

Rather than displaying the icons, unicode strings such as "\u2190 ,\u2192" are displayed.

Checked the between d21e910 and 58b8989. The issue is introduced in the latter commit.

quozl added a commit that referenced this issue Jul 8, 2019
This reverts commit 58b8989.

Reported in #65
@quozl
Copy link

quozl commented Jul 8, 2019

Thanks. I've reproduced, and reverted 58b8989.

@quozl quozl mentioned this issue Jul 8, 2019
@nswarup14
Copy link
Author

@quozl
Thanks. Closing the issue

@quozl
Copy link

quozl commented Jul 10, 2019

Why? Issue still exists, if the Port to Python 3 source code is used. All I've done by reverting is remove the issue from the master branch. We still need the source code ported to Python 3.

@quozl quozl reopened this Jul 10, 2019
@Aniket21mathur
Copy link

Aniket21mathur commented Jul 10, 2019

Thanks. Did some work around.

  • The issue does not happen to me on running the activity with Python 3 version of the toolkit(I checked out 58b8989 in master).
  • The issue takes place only when we run the Python 3 Ported activity with Python 2 version of the toolkit. Which I don't think is the right practice.

To avoid such confusion(at least after the Port to python 3 commit is merged), I think we should do any further changes, compatible with Python 3 and test those with Python 3 version of the toolkit. But since we don't have Python 3 packages yet, we can't scale, so we should wait to have packages before merging "Port to Python 3" to master.

@quozl @nswarup14 What do you suggest?

@Aniket21mathur
Copy link

Aniket21mathur commented Jul 10, 2019

Checked the between d21e910 and 58b8989. The issue is introduced in the latter commit.

@nswarup14 I am still confused how you got this issue. Please help.

@quozl
Copy link

quozl commented Jul 10, 2019

Thanks for diagnosing. It reminds me again how I should not test activities using sugar-activity.

@Aniket21mathur
Copy link

Thanks for diagnosing. It reminds me again how I should not test activities using sugar-activity.

I am confused, did I did anything wrong ? 😅

@quozl
Copy link

quozl commented Jul 10, 2019

Can you think of a way to detect and warn when an activity is run in Python 2 when it has already been ported to Python 3? 😁

@Aniket21mathur
Copy link

I think we already have it, please see sugarlabs/sugar-toolkit-gtk3@aa8a5e7#diff-35db2ad94d2c22d773bce7bb50fdacafR131.

If we have in mind that we are testing for Python 3 activity, and we still get this warning, means something is wrong 😁

We can edit the warning, adding "or you might be using the wrong toolkit version" or something like that 😅 .

@quozl
Copy link

quozl commented Jul 11, 2019

We don't have in mind that we are testing a Python 3 activity. As far as I know, only you and I can do that at the moment, using hand crafted configurations.

@Aniket21mathur
Copy link

We don't have in mind that we are testing a Python 3 activity. As far as I know, only you and I can do that at the moment, using hand crafted configurations.

Thanks, Agreed.

I have something in mind, like somehow reading the Python version from the environment and then comparing it with the command line input, sugar-activity . or sugar-activity3 ..

I will try that and let you know :-)

@rhl-bthr
Copy link

Check if exec in activity/activity.info is consistent with the command being used to run the activity. Raise a warning otherwise

@quozl
Copy link

quozl commented Aug 15, 2019

I've tried cherry-picking 58b8989 and resolving a conflict, but something between then and now has broken it; calculations display no result, and logs contain;

Traceback (most recent call last):
  File "/usr/share/sugar/activities/Calculate.activity/calculate.py", line 859, in keypress_cb
    return f(self)
  File "/usr/share/sugar/activities/Calculate.activity/calculate.py", line 309, in <lambda>
    'Return': lambda o: o.process(),
  File "/usr/share/sugar/activities/Calculate.activity/calculate.py", line 517, in process
    s = numParser.standard(s)
  File "/usr/share/sugar/activities/Calculate.activity/numParser.py", line 41, in standard
    localString = localString.decode('utf-8')
AttributeError: 'str' object has no attribute 'decode'
    def process(self):
        """Parse the equation entered and show the result."""

        s = str(self.text_entry.get_text())
        s = numParser.standard(s)
        label = str(self.label_entry.get_text())

So it seems the new numParser requires unicode data type? Anyone have any ideas?

@Saumya-Mishra9129
Copy link
Member

@quozl The issue is fixed. We forgot to close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants