Some issues that I think of off the top of my head, without looking into the details of the code.
0. I am not sure how I would use this. I am thus not sure why I might push this, especially given that there are a hundred other Idle issues, many with patches that also need to be reviewed. Enlighten and persuade me. Also see 7.
1. There are at least 4 styles of tkinter imports:
a. import tkinter
b. import tkinter as tk
c. from tkinter import a,b,c
d. from tkinter import *
My personal preference is b, c, (a,d). I have never done a census of idlelib modules to see what is currently used. While tkinter was designed somewhat to allow d, 'from *' in general has lost favor since
Idle was written to in favor of b. That change was made in the tkinter docs, for instance
http://docs.python.org/3/library/tkinter.html#a-simple-hello-world-program
a few years ago. So I would like to use 'as tk' for new code.
2. idlever is useless and should probably be removed after checking for any current uses. The import here is not used and should be removed.
3. Writing tests for old code is slow and somewhat tedious. A new module should come with a test module in idle_test/. It might be named test_clipboard.py (see 6.) Check the README and existing files for what I might be looking for.
4. The purpose of the if __name__: block is not clear. If it is simply a sanity check, this should be in the test file, guarded by 'requires(gui)'. The 'if __name__' block should run that file.
5. I dislike the existing and obsolete practice of camel-case module names that duplicate class names. They also violate PEP 8. Uppercase module names outside of idlelib were changed in 3.0. I would like to change the existing names within idlelib also.
6. The practice of putting each class in a separate file is not standard, neither in the stdlib nor in the community at large. This file is under 50 lines, which is pretty small. Some possibilities of where to put this class are the editor file, an new editor helper class file, or a clipboard file. Are there other clipboard classes or functions that could be moved into a consolidated clipboard.py? existing file where this class might
7. As you indicate, this seems more like a proof of concept than a finished design. Let us consider features of the recent files list.
a. The recent files list is not tied to a particular Idle version, let alone a particular window in a particular session of a particular version. I use the save-across sessions and versions features constantly. Most uses I can think of for a clipboard history would involve multiple windows, and possible multiple Idle sessions. An example would be boilerplate 'if __name__'. Perhaps I am more interested in a persistent snippets list with a easy way to copy an entire item (ie, click anywhere on an item and have that copy the entire item to the clipboard to be pasted whereever).
b. The recent files list contains short pointers to possible large files on disk. The clipboard contents are not limited in size. I might not want 10 megabytes kept and saved to disk, so there should be a way to delete entries. Or perhaps entries should be added explicitly rather than automatically.
c. The recent files list is limited to 20 entries -- the 20 most recently opened. (This should be user-configurable.) Entries that are dropped off can be found through the normal open process. Entries that are dropped off a snippet list are gone. So perhaps they should not disappear without warning.
d. Entries in the recent files list fit on one line. That would not be true for snippets. Perhaps a snippets window should use a tree widget so items could be condensed to a single line (the first, or a title) or expanded to the full snippet.
8. In some respects it would be good if this were implemented as an extension that could be enabled or not. See #3068 for improving the selection and configuration of extensions.
9. Little features for Idle can be applied to all versions current at the time of application. See PEP 434 for Idle does not follow the normal rule. But don't worry about the version header.
* (I prefer not to read patches in detail until the author is recorded as having signed the CLA.)
|