Added Tkinter based wrapper GUI #108

ghost · 2013-11-10T05:33:00Z

Finally No single line of core changed.

The GUI is now ready for deployment 😃

Commit 74d66ca

xu-cheng · 2013-11-11T04:27:40Z

courseradownloader/gui.py

+    try:
+        import tkinter as Tk
+    except ImportError:
+        print_('Error: Tkinter module not found: Please use command line version instead')


You should quit the program here.

xu-cheng · 2013-11-11T04:31:33Z

You can add an GUI entry_point in setup.py.

ghost · 2013-11-11T12:26:25Z

@xu-cheng added gui entry point in commit 1935af6

Also changed:
print_('Error: Tkinter module not found: Please use command line version instead')

To:

raise Exception("Error: Tkinter module not found: Please use command line version instead")

xu-cheng · 2013-11-11T12:40:29Z

I think you should set gui entry point like this:

entry_points = { "console_scripts" : [ "coursera-dl = courseradownloader.courseradownloader:main"],
                        "gui_scripts" : ["coursera-dl-gui = courseradownloader.gui:main" ]}

ghost · 2013-11-11T15:12:30Z

@xu-cheng I tried that first but unfortunately if you try that you'll see that the program doesn't work as expected, Upon clicking download the program quits and doesn't print anything

xu-cheng · 2013-11-11T15:17:07Z

You may consider add a log window in your GUI and redirect sys.stdout and sys.stderr into it. Therefore it will be GUI app not console one.

ghost · 2013-11-12T05:28:55Z

That's what I was thinking too...

dgorissen · 2013-11-13T17:42:56Z

Thanks. I had a quick try 2 days ago. From memory:

no window resizing support, you are missing a layout manager it seems (or whatever wx calls them).
ensure the target directory defaults to the current directory (and maybe make show it in the ui somewhere)
I got it to crash but dont remember the steps, will try to replicate.

ghost · 2013-12-23T05:44:15Z

@xu-cheng you are commenting on a outdated commit The latest commit is 5c1d647 Where the issue has been fixed... see above... Also now it Redirects stdout and stderr to a text widget

xu-cheng · 2013-12-23T05:46:54Z

@K-DawG007 Actually I commented the code in the current master branch. It seems that @dgorissen merged wrong commit.

ghost · 2013-12-23T08:17:38Z

@dgorissen oops looks like you merged the wrong commit... BTW @xu-cheng why is this still a open pull request? is that because an outdated commit was merged?

dgorissen · 2013-12-30T21:22:02Z

Not sure what commit you are talking about. I had another look at this pull request. Comments:

On opening the GUI on OSX the bottom buttons are not shown (window too small). You are also missing a layout manager it seems
ensure the target directory defaults to the current directory (and probably best to show it in the ui somewhere)
I get exceptions while downloading a course: Failed to download url XXX... :Std_redirector object has no attribute "flush"
selecting "remember me" does not seem to have any effect, nothing filled in on restart

Minor:

If I just click download without filling in anything I get a TypeError, this could be friendlier
tooltips for the various fields would be friendlier

ghost · 2014-01-01T10:36:33Z

@dgorissen

On opening the GUI on OSX the bottom buttons are not shown (window too small). You are also missing a layout
manager it seems

Well Basically you can't miss a layout manager cause then There'd be no UI (I have used grid). I too noted that on OSX the UI is quite bad So I fixed it in commit fe0e0e1.

selecting "remember me" does not seem to have any effect, nothing filled in on restart

Fixed in commit 0f4fa5d

If I just click download without filling in anything I get a TypeError, this could be friendlier

Fixed....

To Be fixed:

I get exceptions while downloading a course: Failed to download url XXX... :Std_redirector object has no attribute "flush"

I can't seem to reproduce the bug please show how to reproduce....

ensure the target directory defaults to the current directory (and probably best to show it in the ui somewhere)

I don't get what your trying to say please clarify a bit more...

tooltips for the various fields would be friendlier

I'll try to add that feature :)

xu-cheng · 2014-01-01T10:48:11Z

I get exceptions while downloading a course: Failed to download url XXX... :Std_redirector object has no attribute "flush"

I can't seem to reproduce the bug please show how to reproduce....

This will happen when you download a large file. In this case, in CLI, a download status would be appeared to show you speed and progress.

So basically,in GUI, your Std_redirector class need support these:

When writing '\r' to GUI, current line will be removed.
Support flush() function. I use sys.stdout.flush() in CLI to force it refresh.

BTW, you need sync your pull request with upstream.

ghost · 2014-01-01T15:42:25Z

@xu-cheng I reckon its fixed now...

dgorissen · 2014-01-01T21:15:10Z

Thanks, already improved. Comments:

ensure the target directory defaults to the current directory (and probably best to show it in the ui somewhere)
I don't get what your trying to say please clarify a bit more...

The gui does not show you what the target directory is. IMHO would make things clearer for the user (he can see what is set by default and catch mistakes if he selected the wrong folder). Typically you see this done with a line edit - browse button combo but there are different options.

If I just click download without filling in anything I get a TypeError, this could be friendlier

Even better would be to see the error message in the GUI itself :) Just a tip, you can also ensure you only enable the download button once the course name has been filled in.

gzip/reverse sections: why two radio buttons for each? Just use a checkbox.
I also suggest grouping the optional/advanced settings to its own (preferably collapsible) panel or tab. Note not everybody may understand what the ** means.
I would show the version number of coursera-dl somewhere (titlebar?). Always helps with debugging

ghost · 2014-01-02T17:41:04Z

@dgorissen thanks for the clarification, Now it shows the default directory

If I just click download without filling in anything I get a TypeError, this could be friendlier

Now the Download button is enabled after a course name is entered.

gzip/reverse sections: why two radio buttons for each? Just use a checkbox.

Fixed....

I would show the version number of coursera-dl somewhere (titlebar?). Always helps with debugging

Added feature...

To be fixed:

I also suggest grouping the optional/advanced settings to its own (preferably collapsible) panel or tab. Note not everybody may understand what the ** means.

Tough one BTW I'll try to add that feature....

Also it says I have edited ReadMe.md and courseradownloader.py when I really haven't how can I fix this so merging becomes easier? I tried something stupid with commit 3bd383d but that didn't workout too, please help I am not that much of an git expert...

dgorissen · 2014-01-02T22:28:26Z

BTW, note also commit 6c7fbcf. Will want to update your code as well (or ideally) avoid the duplication.

ghost · 2014-01-03T06:28:16Z

@dgorissen thanks added code from commit 6c7fbcf and hopefully merging should now be easier :)

xu-cheng · 2014-01-03T10:29:16Z

@K-DawG007 I think you handled the flush in wrong way. Also you didn't process '\r' well. You may find this helpful.

ghost · 2014-01-04T11:19:11Z

@xu-cheng Well mimicking that effect is quite hard in a Tkinter Text widget So I simply used a Label widget to display the status in the lower left area... any comments?

xu-cheng · 2014-01-04T12:07:22Z

@K-DawG007 Sorry, I have no experience in GUI programming. So I cannot offer too much help.

Also I found a new bug in your code.

try:
    import Tkinter as Tk
    import ttk
    import tkFileDialog
    import tkMessageBox

except ImportError:
    try:
        #In python 3 the module Tkinter was renamed to tkinter
        import tkinter as Tk

    except ImportError:
        raise Exception("Error: Tkinter module not found use command line version instead")

You didn't import ttk, tkFileDialog and tkMessageBox in python 3.

In fact you can just import tk using six module, it will handle python 2 and 3 well. Doc: http://pythonhosted.org/six/#module-six.moves

ghost · 2014-01-05T05:58:02Z

@xu-cheng Thanks I actually never tested that part, BTW its fixed now however if you get a
ImportError: cannot import name tkinter_ttk you need to update your six module.

I suggest you test the gui now....the flush() issue has been resolved

xu-cheng · 2014-01-05T06:10:17Z

@K-DawG007 good job!

Here's a minor suggestion. I recommend you change 'status' in string to string.startswith('status:'), and change ' '*25 in string to string == ' ' * 25 + '\r'. So you can get better robust. In case user may download a file whose name contains 'status'.

ghost · 2014-01-05T12:40:27Z

@xu-cheng Thanks for the suggestion :)

@dgorissen I suggest you update the current gui with this updated gui cause there's practically no way to launch the gui in your branch plus there are a ton of bugs in that.

Also I suggest updating the readme stating the launching of the gui via coursera-dl-gui plus maintaining a gui version number if possible

I'll try to replace the ** with a optional/advanced settings and also I'll' give a shot at the tooltips. Help is always appreciated though

dgorissen · 2014-01-06T17:32:08Z

@K-DawG007 mmm, I had not realized I had merged gui.py. That was unintended and by accident, was planning to wait until it reached a stable state. I will leave it there for now and merge your pull request after one more round of testing. Let me know once you are fully happy with it.

Btw, I dont see the real need for an extra confirmation dialog when you click on "Download". What was the rationale?

Minor: be consistent in using capitalized labels for buttons and menus (e.g., Quit instead of quit)

dgorissen · 2014-01-15T11:57:03Z

Any progress on this, would like to put out a release.

ghost · 2014-01-16T13:01:28Z

I'm currently facing a exam so expect a bit of delay

will try to work on this when I have time though :)

ghost · 2014-06-06T11:11:44Z

What's Changed:

Progress bar added instead of displaying text status.
The extra confirmation dialog has now been removed (why did I put it anyway?? silly me)

To be added:

Tooltips when a user hovers over a textbox
Something more? any ideas??

And please could some linux user post some screenshots of this?

Added full python 3 support

2e8ab24

xu-cheng reviewed Nov 11, 2013
View reviewed changes

Added GUI entry point

1935af6

Redirects stdout and stderr to a text widget

5c1d647

K-DawG007 added 2 commits December 31, 2013 23:13

Fixed several minor bugs

0f4fa5d

Fixed window size

fe0e0e1

Fixed bug when downloading large files

111ebd2

K-DawG007 added 2 commits January 2, 2014 23:13

Added more features

c49a1b8

merged with original repo

3bd383d

K-DawG007 added 2 commits January 3, 2014 11:26

fixed merge issue

808a6a7

added code from commit 6c7fbcf

b26506d

Fixed bugs and added save to log feature

4657f68

Minor enhacements and bug fixes

0d893fe

Minor enhancements

4370b4e

Added Collapsible Panel

74d66ca

unknown added 2 commits June 6, 2014 16:26

Added progress bar and minor improvements

b8e5e00

fixed minor issue with ReadMe

0e886d8

Removed a debug line

e1badef

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Tkinter based wrapper GUI #108

Added Tkinter based wrapper GUI #108

ghost commented Nov 10, 2013

xu-cheng Nov 11, 2013

xu-cheng commented Nov 11, 2013

ghost commented Nov 11, 2013

xu-cheng commented Nov 11, 2013

ghost commented Nov 11, 2013

xu-cheng commented Nov 11, 2013

ghost commented Nov 12, 2013

dgorissen commented Nov 13, 2013

ghost commented Dec 23, 2013

xu-cheng commented Dec 23, 2013

ghost commented Dec 23, 2013

dgorissen commented Dec 30, 2013

ghost commented Jan 1, 2014

xu-cheng commented Jan 1, 2014

ghost commented Jan 1, 2014

dgorissen commented Jan 1, 2014

ghost commented Jan 2, 2014

dgorissen commented Jan 2, 2014

ghost commented Jan 3, 2014

xu-cheng commented Jan 3, 2014

ghost commented Jan 4, 2014

xu-cheng commented Jan 4, 2014

ghost commented Jan 5, 2014

xu-cheng commented Jan 5, 2014

ghost commented Jan 5, 2014

dgorissen commented Jan 6, 2014

dgorissen commented Jan 15, 2014

ghost commented Jan 16, 2014

ghost commented Jun 6, 2014

Added Tkinter based wrapper GUI #108

Are you sure you want to change the base?

Added Tkinter based wrapper GUI #108

Conversation

ghost commented Nov 10, 2013

xu-cheng Nov 11, 2013

Choose a reason for hiding this comment

xu-cheng commented Nov 11, 2013

ghost commented Nov 11, 2013

xu-cheng commented Nov 11, 2013

ghost commented Nov 11, 2013

xu-cheng commented Nov 11, 2013

ghost commented Nov 12, 2013

dgorissen commented Nov 13, 2013

ghost commented Dec 23, 2013

xu-cheng commented Dec 23, 2013

ghost commented Dec 23, 2013

dgorissen commented Dec 30, 2013

ghost commented Jan 1, 2014

To Be fixed:

xu-cheng commented Jan 1, 2014

ghost commented Jan 1, 2014

dgorissen commented Jan 1, 2014

ghost commented Jan 2, 2014

dgorissen commented Jan 2, 2014

ghost commented Jan 3, 2014

xu-cheng commented Jan 3, 2014

ghost commented Jan 4, 2014

xu-cheng commented Jan 4, 2014

ghost commented Jan 5, 2014

xu-cheng commented Jan 5, 2014

ghost commented Jan 5, 2014

dgorissen commented Jan 6, 2014

dgorissen commented Jan 15, 2014

ghost commented Jan 16, 2014

ghost commented Jun 6, 2014

What's Changed:

To be added: