Fixes

[JuiP]

• Undo and Redo buttons were hidden. Implemented methods such that the buttons are set sensitive only when required. For now one can only undo and redo one step at a time.

• I don't know how exactly paste and copy buttons work. If not required should either be hidden / removed.

@quozl @chimosky Please test and review :)

[Hrishi1999]

Tested, haven't reviewed. I don't really notice anything when I click on undo or redo. I get the following traceback when I click on undo, then on redo (once undo button has been disabled)

  File "/home/hrishi/wordcloud/activity.py", line 427, in _redo_cb
    text_buffer = self.save_text_redo.pop()
IndexError: pop from empty list

The paste button seems to paste text from the clipboard to the "Cloud Text". The copy button, however, doesn't copy the "Cloud Text"

[JuiP]

Tested, haven't revied. I don't really notice anything when I click on undo or redo.

Notice Cloud text. It will be updated with your previously entered text.
For testing -

• Write few words in cloud text and then click Create the cloud
• At this point you will notice that the undo and redo button are disabled, because there is nothing to be undone.
• Write new set of words in cloud text and then click Create the cloud
• The undo button is now enabled and on clicking undo, you can see a change in words in cloud text.
• You can now either click on Create the cloud or on the redo button
• When the redo button is clicked, notice the change in words in cloud text. Also now, both the undo and redo button will be disabled because this feature lets you undo and redo once, it can later be extended to provide support for multiple undo and redo. Now the text you enter in cloud text is treated as a new list so repeate from first bullet point

File "/home/hrishi/wordcloud/activity.py", line 427, in _redo_cb
text_buffer = self.save_text_redo.pop()
IndexError: pop from empty list

For Redo I had assumed the user will create a cloud before redoing, if one tries to click redo before creating a cloud then an IndexError is seen. Didn't realize it while testing because I always clicked Create the cloud after undo. Nevermind, thanks @Hrishi1999..... Done in a32c102

[Hrishi1999]

Tested. No issues now.

[quozl]

Tested 13bafe5 on Ubuntu 20.04. Failed to generate a new word cloud;

$ sugar-activity3
pygame 1.9.6
Hello from the pygame community. https://www.pygame.org/contribute.html
Traceback (most recent call last):
  File "/usr/share/sugar/activities/WordCloud.activity/activity.py", line 379, in _create_image
    subprocess.check_call(
  File "/usr/lib/python3.8/subprocess.py", line 359, in check_call
    retcode = call(*popenargs, **kwargs)
  File "/usr/lib/python3.8/subprocess.py", line 340, in call
    with Popen(*popenargs, **kwargs) as p:
  File "/usr/lib/python3.8/subprocess.py", line 854, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.8/subprocess.py", line 1702, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: '/usr/share/sugar/activities/WordCloud.activity/wordcloud.py'
$ 

Because there is no such file /usr/bin/python on Ubuntu 20.04.

[quozl]

Thanks. Tested 9ee9b2f on Ubuntu 20.04. Failed to generate a new word cloud, same stack trace.

[chimosky]

@JuiP the text doesn't get copied to the clipboard as copy_clipboard copies the currently-selected text to a clipboard but you should do that only when there's a text selection.

[JuiP]

Tested 5f32c8a. Thanks! @chimosky

Because there is no such file /usr/bin/python on Ubuntu 20.04.

To resolve this, I was looking at the comments on #441. Is this issue due to this line in setup.py? @quozl

[quozl]

I think that is likely; just test it and see? Write a short program that calls another short program using subprocess module, and set the first line to something that doesn't exist, like /usr/bin/pythonmissing

If the theory is correct, you'll see a near-identical traceback.

[JuiP]

Tried fixing it in c8bffd4. From what I have found, Ubuntu 20.04 only has python3 installed by default. So, /usr/bin/python cannot be used.

I also changed #!/usr/bin/python to #! /usr/bin/env python3 , so if the user has several versions of Python installed, /usr/bin/env will ensure the interpreter used is the first one on the user environment's $PATH. The alternative would be to hardcode something like #!/usr/bin/python3. But that is less flexible.
@quozl Could you please have a look at the changes and test? :)

[chimosky]

Tried fixing it in c8bffd4. From what I have found, Ubuntu 20.04 only has python3 installed by default. So, /usr/bin/python cannot be used.

I also changed #!/usr/bin/python to #! /usr/bin/env python3 , so if the user has several versions of Python installed, /usr/bin/env will ensure the interpreter used is the first one on the user environment's $PATH. The alternative would be to hardcode something like #!/usr/bin/python3. But that is less flexible.
@quozl Could you please have a look at the changes and test? :)

Reviewed c8bffd4, thanks.

Using #!/usr/bin/env python uses whatever version of python that's the default, so on a machine with just python3, it'll get executed.
I agree, hardcoding #!/usr/bin/python3 isn't a good idea.

[Saumya-Mishra9129]

Tested on ubuntu 20.04, I haven't get this one error.

Tried fixing it in c8bffd4. From what I have found, Ubuntu 20.04 only has python3 installed by default. So, /usr/bin/python cannot be used.

I also changed #!/usr/bin/python to #! /usr/bin/env python3 , so if the user has several versions of Python installed, /usr/bin/env will ensure the interpreter used is the first one on the user environment's $PATH. The alternative would be to hardcode something like #!/usr/bin/python3. But that is less flexible.

This is kind of new info for me. Lets see if it fixes https://github.com/sugarlabs/sugar-toolkit-gtk3/issues/441.

However I found errors in log while creating cloud, there are errors along with MEMORY ERROR.

[JuiP]

Tested on ubuntu 20.04, I haven't get this one error.

Great! 😎
Since I don't have a ubuntu 20.04 system to test my changes, could you please confirm if c8bffd4 fixed the error @quozl mentioned? The way to check that can be
git checkout 5f32c8a So you will be in a detached head state. You can later update your test branch by git pull

However I found errors in log while creating cloud, there are errors along with MEMORY ERROR.

Thanks! @Saumya-Mishra9129 ... Just realized the error you got was due to an incomplete python3 port. I have fixed it.

Also, this time while testing I got DepecationWarning: Gdk.Screen.height is deprecated I haven't seen this warning before in any activity I tested. https://lazka.github.io/pgi-docs/Gdk-3.0/classes/Screen.html says use per-monitor information

[Saumya-Mishra9129]

Also, this time while testing I got DepecationWarning: Gdk.Screen.height is deprecated I haven't seen this warning before in any activity I tested. https://lazka.github.io/pgi-docs/Gdk-3.0/classes/Screen.html says use per-monitor information

Yeah, I also searched about it. You can use Gdk.Monitor class for same as per doc https://lazka.github.io/pgi-docs/Gdk-3.0/classes/Monitor.html#Gdk.Monitor.get_scale_factor. But I don't think we need to fix that because its deprecated in Gdk 3.22 , it is not removed , we can still use Gdk.Screen because we have used it in several activities also.

Great! sunglasses
Since I don't have a ubuntu 20.04 system to test my changes, could you please confirm if c8bffd4 fixed the error @quozl mentioned? The way to check that can be
git checkout 5f32c8a So you will be in a detached head state. You can later update your test branch by git pull

Checked didn't get error now. It is fixed , but its different from sugarlabs/sugar-toolkit-gtk3#441 , I checked same thing you said in findwords activity , It didnt work there.

[chimosky]

@JuiP said

Also, this time while testing I got DepecationWarning: Gdk.Screen.height is deprecated I haven't seen this warning before in any activity I tested. https://lazka.github.io/pgi-docs/Gdk-3.0/classes/Screen.html says use per-monitor information

I agree with @Saumya-Mishra9129, you'll get a warning that it's deprecated but it still exists and hasn't been removed yet so you can keep using it.

[JuiP]

If @quozl approves the changes and confirms there aren't anymore errors, maybe I could go ahead with the changes for making a release? The activity has been ported to python3 after the last release, so I guess we can have this activity released?

[chimosky]

Did some changes and released the last python2 version, also resolved conflicts on this branch.

[JuiP]

Updated my local branch with current changes! 👍

[JuiP]

@quozl @chimosky I guess there are no more changes....can we merge this PR too?

[quozl]

Thanks. Tested 3a6dcfb on Ubuntu 20.04.

• the text entry has no cursor, (possibly this is a Sugar toolkit CSS problem),
• some of the words do not fit on the display,
• logs contain DeprecationWarnings for Gdk.Screen.height and Gdk.Screen.width.

[JuiP]

@quozl I'll look into the issues you mentioned and get back,
howerever for

• logs contain DeprecationWarnings for Gdk.Screen.height and Gdk.Screen.width.

Should we just ignore the warning, like suggested by @chimosky and @Saumya-Mishra9129 in their comments? (here and here)

[quozl]

Should we just ignore the warning, like suggested by @chimosy and @Saumya-Mishra9129 in their comments? (here and here)

My opinion is;

• for deprecated APIs that cause no warnings, no action needs to be taken,
• for warnings, these cost CPU time and disk space in logs, and the effect on the planet scales with the number of Sugar users, so some action should be taken, and there will be a return on investment,
• for this particular deprecation, perhaps write a function to be reused,

def get_display_rectangle():
    display = Gdk.Display.get_default()
    monitor = display.get_primary_monitor()
    geometry = monitor.get_geometry()
    scale_factor = monitor.get_scale_factor()
    width = scale_factor * geometry.width
    height = scale_factor * geometry.height
    return (width, height)

However, in most activities that use Gdk.Screen in this way they do so in order to avoid having to wait for the main window to be realised, and are assuming that the main window will take up the whole screen because of Sugar's window manager configuration. Instead, activities should be coded to get the dimensions of the main window, and that way they will be independent of Sugar's window manager.

[JuiP]

the text entry has no cursor, (possibly this is a Sugar toolkit CSS problem),

@quozl I guess your speculation is correct. I tried setting Gtk.TextView.set_cursor_visible(True) It had no effect so I'm guessing it is indeed Sugar toolkit issue.

[JuiP]

@quozl @chimosky @srevinsaju Kindly test, review and merge?

[quozl]

@quozl I guess your speculation is correct. I tried setting Gtk.TextView.set_cursor_visible(True) It had no effect so I'm guessing it is indeed Sugar toolkit issue.

Can you fix it then? You can find the CSS files in the sugar-artwork repository.

[JuiP]

Sure, I'll look into how to get that fixed; till then can we merge this PR so it can have a python3 release? @quozl @chimosky

[chimosky]

Reviewed, thanks.