Repository navigation
Add turtledemo to IDLE menu #61374
Description
| BPO | 17172 |
|---|---|
| Nosy | @rhettinger, @terryjreedy, @kbkaiser, @ronaldoussoren, @orsenthil, @ned-deily, @ezio-melotti |
| Dependencies | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/terryjreedy'
closed_at = <Date 2014-07-29.02:41:07.634>
created_at = <Date 2013-02-09.20:32:46.152>
labels = ['expert-IDLE', 'type-feature']
title = 'Add turtledemo to IDLE menu'
updated_at = <Date 2014-07-29.02:41:18.094>
user = 'https://github.com/rhettinger'bugs.python.org fields:
activity = <Date 2014-07-29.02:41:18.094>
actor = 'terry.reedy'
assignee = 'terry.reedy'
closed = True
closed_date = <Date 2014-07-29.02:41:07.634>
closer = 'terry.reedy'
components = ['IDLE']
creation = <Date 2013-02-09.20:32:46.152>
creator = 'rhettinger'
dependencies = ['21597', '21823', '21824']
files = ['29048', '35328', '35383', '36081', '36121']
hgrepos = []
issue_num = 17172
keywords = ['patch']
message_count = 35.0
messages = ['181757', '181780', '181781', '181954', '181955', '182160', '182208', '192929', '218983', '219026', '219064', '219237', '219243', '219245', '219246', '219252', '219318', '219330', '219538', '220413', '221220', '222815', '223913', '224026', '224091', '224098', '224154', '224161', '224176', '224181', '224206', '224209', '224213', '224214', '224215']
nosy_count = 11.0
nosy_names = ['rhettinger', 'terry.reedy', 'kbk', 'ronaldoussoren', 'orsenthil', 'ned.deily', 'ezio.melotti', 'jesstess', 'python-dev', 'Ramchandra Apte', 'Lita.Cho']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17172'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']
Activity
rhettinger commentedon Feb 9, 2013
The turtledemo is an on-ramp for younger programmers and we should make it easy to launch.
RamchandraApte commentedon Feb 10, 2013
Will attach patch. Coincidentally I'm am a younger programmer.
RamchandraApte commentedon Feb 10, 2013
Should be "... I'm a younger..."
RamchandraApte commentedon Feb 12, 2013
Attached is a patch. I hope the File menu is the right place for this. I had to move the code in Lib/turtledemo.py after "if __name__ ==..." into main().
RamchandraApte commentedon Feb 12, 2013
Should this be added to Lib/idlelib/NEWS.txt ?
ezio-melotti commentedon Feb 15, 2013
I left a comment in rietveld.
I think that's the best place where to put it.
Why?
Yes.
RamchandraApte commentedon Feb 16, 2013
Because turtledemo doesn't have a main() function. I moved the code under 'if __name__ == "__main__"' into a main() function.
rhettinger commentedon Jul 12, 2013
Ezio, would you like to take it from here?
LitaCho commentedon May 23, 2014
I tested the patch and it looks correct upon inspection. It looks like it applies cleanly and a straight forward solution.
I made a slight change so that when the Demo exits, it has a better message. I also added the change to the NEWS.txt file.
I also ran the IDLE tests, and everything passed. I didn't see new tests added, but I am not sure if that is needed, since we are just adding a menu binding.
I've attached my patch with the changes listed above. It should be ready for review.
terryjreedy commentedon May 24, 2014
My thoughts on this. Overall, +.something (and increasing as I look at it more ;-).
I am opposed to adding this to the relatively long file menu. It has nothing to do with manipulating or editing files. If it is to go in, please put it on the help menu, at least for now, as the intention is to somehow help beginners. If we ever add a demo section or submenu to the Help menu or Demo to the top-level level menu, we can move this.
Any patch should be applied to both 3.4 and 3.5. Turtledemo apparently does not exist in 2.7.
Idle NEWS items first go into the Idle section of the regular NEWS file. They are supposed to be copied into the idlelib/NEWS with each release. That has sometimes happened but not always. (I should open separate issue to fix both the idlelib files and the procedure.) In any case, NEWS entries should not be added to a patch until it is about to be committed. Otherwise, merge conflicts are possible or even likely.
With the main code in turtledemo.__main__ wrapped in a function, we could pass an 'idle' argument so that the demo could modify its behavior when run from Idle. I mention a possibility below.
The patch runs turtledemo within the Idle process. It might be better to run it in a separate subprocess:
python -c "from turtledemo.__main__ import main; main('idle')"
One reason, aside from not leaving all the imports around, is that 3 of the demos print to console.
When Idle is run windowless with pythonw on Windows, attempts to print to the non-existent console crash the process. There have been several bug issues about this. The current patch should not be pushed with turtledemo as is unless tested with installed 3.4.1 by hand-patching idlelib. Or, all console prints should be eliminated.
However, a future addition to turtledemo could create a problem. That is why I think it better that only idlelib code run within the idle process.
I am bothered a bit that there is little relationship between Idle and turtledemo. Both using tkinter seems not too relevant. That both tend to be used by less experienced Python users seems a bit thin. The relationship would be stronger if there were other demos (which I would like) or if there were option in the demo to load the code for a particular example into an Idle editor window. (Each demo will run independently in an Idle user process - text output, including tracebacks resulting from ungracefully closing an example before it is done, appears in the Shell window.)
LitaCho commentedon May 24, 2014
I did not know that NEWS items should not be edited unless it is about to be committed. The previous comments suggested to put it in. Thank you for the feedback.
I can move the Turtle Demp into the Help Menu rather than the File menu. I agree that it seems out of place. I can also make it such that it runs the demo as an external process rather than within IDLE. That way, it is completely separate by IDLE and we don't create a dependency.
I think if we were to load in future demos, they could be run as other processes rather than within IDLE, so that future additions don't end up crashing IDLE.
14 remaining items
LitaCho commentedon Jul 26, 2014
I wasn't aware of make patchcheck. I will run this script when submitting patches in the future. Thanks, Ned!
LitaCho commentedon Jul 27, 2014
I've updated this patch to include the changes Ned mentioned. I am waiting to hear from Ronald if he has a better solution about dealing with the focus problem with the keyboard and mouse.
ned-deily commentedon Jul 28, 2014
turtle_demo_v3.patch looks good to me. I have satisfied myself that the AppleScript works OK with a non-English system. And, while it is a bit of a kludge for OS X and assuming the rest of the change works OK on Windows and Linux, I would recommend to Terry that we proceed with this approach for 3.5, 3.4 if someone feels strongly about it, and skip 2.7. If a better way to do it turns up, we can add that later. Time to move on to other things.
ronaldoussoren commentedon Jul 28, 2014
I don't have a better solution for this, although I'd slightly prefer to add a hack to the turtledemo main function to force itself to the foreground on startup. That way the hack also works when the user starts the script in a Terminal window.
LitaCho commentedon Jul 28, 2014
Thanks for the input Ronald! How would I go about forcing the turtledemo to be in the foreground? Do I just need to call 'fg' on the subprocess? I've been Googling and couldn't find anything obvious.
ned-deily commentedon Jul 28, 2014
Ronald:
That's exactly what the proposed patch does. The AppleScript "activate" causes turtledemo, however started, to move to the foreground.
terryjreedy commentedon Jul 29, 2014
I agree with skipping 2.7. The current patch is based on importing turtledemo, while on 2.7, turtleDemo.py is in Demo/turtle/ and cannot be imported. It could run with execfile, but the viewer will only run if /turtle is the current directory. On Windows, this would still be useless as the 2.7 installer does not install Demo or anything therein. When turtleDemo is installed, the location is system dependent. An extended version of the new checker extension, bpo-21880, would allow users to configure 'run turtledemo' to work on a system that has it.
The patch works fine on Windows, including not blocking other actions with Idle. The only change I might make is
cmd = [sys.executable, '-m', 'turtledemo']
which is more readable and does the same as the current version. But see below. (Lita, don't make a new patch for this or anything else in this message.)
The main remaining demo viewer fix is font re-sizing, bpo-21933, which I expect we should finish within a week.
'Run turtledemo' should go on the Run menu, along with other 'run <external program>' menu items. The Run menu is easily enabled in the Shell window by adding '("run", "_Run"),' to menu_specs before options (line 842). However, putting the entry where it would belong, after Run Module, means that the addition has to be delayed until after the ScriptBinding extension is loaded. That would be done most easily as part of loading the checker extension bpo-21880. The open_turtle_demo function would be moved and perhaps generalized.
The change to make turtledemo run properly in a subprocess on Mac appears to be a separate issue from adding it to Idle's menu. I presume that if turtledemo.__main__ is loaded into Idle, and run with F5, as I have been doing, there is the same problem (on a Mac). I would guess that the same would be true of any tkinter app run in a subprocess from Idle. How about non-gui apps that are not mac-aware? If any of these are true, the fix would be generally useful outside of turtledemo and would seem to belong somewhere in the tkinter package. (Is the same true for some of maxosxSupport).
If the activate code is moved to macosxSupport.activate(), does the following work in EditorWindow.open_turtle_demo?
cmd = [sys.executable, '-c',
'from idlelib.macosxSupport import activate as A; A()\n'
'from turtledemo.__main__ import main; main()']
Does the activate code have to be called after, say, the tkinter import in turtledemo, or is anytime in the process ok? While something like this is not needed to run turtledemo, which we can edit, it would be to run tkinter (or mac-unaware gui apps) that we cannot edit.
ned-deily commentedon Jul 29, 2014
The activate code has to be called in the subprocess after Tk() has been called because only then is the subprocess guaranteed to have been promoted to an OS X GUI process. So that suggestion will not work. The idea of adding something to tkinter was discussed in msg192523 of bpo-11571, though not this specific solution. If you want to pursue more generalized issues of GUI application behaviors on OS X, that should be the subject of another issue. I suggest that this issue remain focused on the original topic. I also would be concerned about adding the Run menu to the Shell window (or system menu bar when the Shell window is active) merely to accommodate this feature. Having turtledemo in the Help menu seems fine to me.
python-dev commentedon Jul 29, 2014
New changeset 04dd26ca02f4 by Terry Jan Reedy in branch '3.4':
Issue bpo-17172: Add the ability to run turtledemo from Idle.
http://hg.python.org/cpython/rev/04dd26ca02f4
terryjreedy commentedon Jul 29, 2014
I would have made the turtledemo fix a separate issue. Given the combined patch, I will just make separate NEWS entries (which I am working on now).
I expect there to be other new entries for Shell/Run, and will move when that is true.
python-dev commentedon Jul 29, 2014
New changeset e344539cda11 by Terry Jan Reedy in branch '3.4':
Issue bpo-17172: add NEWS
http://hg.python.org/cpython/rev/e344539cda11