This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2019年08月21日 01:54 by GeeTransit, last changed 2022年04月11日 14:59 by admin. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 15368 | merged | GeeTransit, 2019年08月21日 21:11 | |
| PR 15689 | merged | miss-islington, 2019年09月05日 01:35 | |
| PR 15690 | merged | miss-islington, 2019年09月05日 01:35 | |
| Messages (14) | |||
|---|---|---|---|
| msg350043 - (view) | Author: George Zhang (GeeTransit) * | Date: 2019年08月21日 01:54 | |
I've just started using IDLE's module/path browsers and they offer a lot! Putting aside the issue of them opening in separate windows, they have a small change that could be made to improve them. Both browsers have scrollbars, but (for me at least) I cannot scroll using my mouse. I propose adding support for scrolling similar to the editor/shell windows. |
|||
| msg350047 - (view) | Author: Terry J. Reedy (terry.reedy) * (Python committer) | Date: 2019年08月21日 03:58 | |
I agree. We added mousewheel scrolling to editor just over a year ago and later added it to text views. But for the browsers, I want to factor out the common code. It is a bit tricky since the 3 major systems each send different events for the same action, and macOS has the opposite convention for how the text moves when pushing the wheel up. |
|||
| msg350048 - (view) | Author: Terry J. Reedy (terry.reedy) * (Python committer) | Date: 2019年08月21日 04:03 | |
#31461 is the index issue for class browser. Mousewheel scrolling was listed without an issue. Now there is, and this has been added as a dependency. |
|||
| msg350095 - (view) | Author: George Zhang (GeeTransit) * | Date: 2019年08月21日 16:44 | |
I looked at the code for scrolling and moved it over to the ScrolledCanvas and TreeNode (because it uses a Label that sits on the canvas, meaning we have to rebind it here). I haven't figured out how to add the scroll-by-pressing-down-and-moving way but I'll look further into it. About the factoring out part, the ScrolledCanvas and TreeNode are both used by the two browsers, meaning that no code has to be added to them. In the future, a factory function could be made that when called with a canvas, it returns an event callback function that can be bound to the canvas. When called, it scrolls depending on the event type and other info. |
|||
| msg350110 - (view) | Author: George Zhang (GeeTransit) * | Date: 2019年08月21日 21:16 | |
Looks like my PRs are getting out of hand... This is the final PR :P |
|||
| msg350113 - (view) | Author: Terry J. Reedy (terry.reedy) * (Python committer) | Date: 2019年08月21日 21:41 | |
I won't merge with mousescroll duplicated, or worse, triplicated. If 'self.text/canvas' is replaced with 'event.widget', then the 'self' parameter can be deleted and the function made a standalone module function. For now, put it in tree, copied with the docstring from editor.py. Then import it into editor. (Reason: Avoid creating more import loops.)
This will make mousescroll depend on IDLE not subclassing text/canvas and overriding yview_scroll. It currently does not and I expect it never will. But, to be safe, this should be added to the docstring, and tested (fairly simple) along with other added tests.
The labels partially blocking the canvas bindings is nasty. Mousescroll is wrapped as a tk script for each label. I expect to eventually replace the labels and other visual tree stuff with a ttk.Treeview. Then no canvas wheel bindings will be needed. In anticipation of that, replace 'yview_scroll(' with the equivalent 'yview(SCROLL,' (Treeview only has the latter.) The resulting line will be
event.widget.yview(SCROLL, lines, "units")
For some reason, creating a module browser for pyshell takes 3 times as long with my repository 3.9 debug build as with my 3.8 installed normal build. (The ration is usually about 2.) But the patch with the extra bindings and wrappings does not make it much worse, if any.
Scrolling by moving mouse while holding down middle mouse wheel seems to be built into Text. But I never/seldom use it and have not tried to add it to anything else. At least on Windows, it works differently than on Firefox. Text only moved with drag, which makes it jerky, not with distance from start position. And one cannot only scroll part of a large file, not to top and bottom. Notepad and notepad++ do not have it. So skip it for now.
When one edits a branch and pushes commits to ones github fork, the changes appear on any existing PR. Closing a PR and reopening a new one is unusual and loses comments. Post to core-mentorship if you need help with git and our workflow. My PR-15360 comment applies to the current one.
|
|||
| msg350210 - (view) | Author: George Zhang (GeeTransit) * | Date: 2019年08月22日 16:24 | |
I renamed mousescroll to handlescroll as it's an independent callback function and I think it fits its use case better. I can keep it as mousescroll if you like though. The handlescroll function is now a standalone module function in tree.py and the EditorWindow imports it for use (instead of creating its own function). Its signature is `handlescroll(event, widget=None)` where event is the event (yeah...) and widget is an optional argument that overrides the widget to call `yview(SCROLL, lines, "units")` on. The second argument was added so that the nasty labels don't have to use the same function but with one line changed (we redirect handlescroll to use `self.canvas` because `event.widget` would refer to the Label which has no yview function). I've added tests on handlescroll with different events. I've also added a test on multicall that checks to test if MultiCallCreator overrides yview. Sorry about the PR closing and reopening. I was panicking about commiting more than once and saw that I should make a new branch for the patch (instead of using master branch). |
|||
| msg350216 - (view) | Author: George Zhang (GeeTransit) * | Date: 2019年08月22日 17:49 | |
Also, how should I get the new code coverage percentage (or should it be ignored for now)? I'm thinking of adding a few more tests that send invalid events which would raise KeyError but I don't think that this behaviour will be used (and it's not documented). Should it give a more specific error message? The test for multicall is only targeting the MultiCall class that gets returned. How should it check for any possible subclasses of it? |
|||
| msg350222 - (view) | Author: Terry J. Reedy (terry.reedy) * (Python committer) | Date: 2019年08月22日 18:21 | |
'mousescroll' was not exact because the mouse is also used to scroll with the scrollbar. 'handlescroll' is worse. 'wheelscroll' seems awkward. 'scrollwheel' (scroll with the mouse wheel) is specific. At least in idlelib, event handlers are routinely called something_event, so use 'wheel_event'. Pressing the wheel is a Button-3 event (there used to be 3-button mice before wheeels) and a handler for that would be 'button3_event' or 'button3_press_event'. |
|||
| msg350223 - (view) | Author: Terry J. Reedy (terry.reedy) * (Python committer) | Date: 2019年08月22日 18:22 | |
Don't worry more about tests until I look at what you have done already. |
|||
| msg351166 - (view) | Author: Terry J. Reedy (terry.reedy) * (Python committer) | Date: 2019年09月05日 01:33 | |
New changeset 2cd902585815582eb059e3b40e014ebe4e7fdee7 by Terry Jan Reedy (GeeTransit) in branch 'master': bpo-37902: IDLE: Add scrolling for IDLE browsers. (#15368) https://github.com/python/cpython/commit/2cd902585815582eb059e3b40e014ebe4e7fdee7 |
|||
| msg351171 - (view) | Author: miss-islington (miss-islington) | Date: 2019年09月05日 01:53 | |
New changeset 9c2654d1aa85968fede1b888fba86aebc06c5be6 by Miss Islington (bot) in branch '3.8': bpo-37902: IDLE: Add scrolling for IDLE browsers. (GH-15368) https://github.com/python/cpython/commit/9c2654d1aa85968fede1b888fba86aebc06c5be6 |
|||
| msg351172 - (view) | Author: miss-islington (miss-islington) | Date: 2019年09月05日 01:58 | |
New changeset 16af39aa84cc3553c51d57461964ab4e28029184 by Miss Islington (bot) in branch '3.7': bpo-37902: IDLE: Add scrolling for IDLE browsers. (GH-15368) https://github.com/python/cpython/commit/16af39aa84cc3553c51d57461964ab4e28029184 |
|||
| msg351174 - (view) | Author: Terry J. Reedy (terry.reedy) * (Python committer) | Date: 2019年09月05日 04:13 | |
Thanks for the patch. More IDLE patches would be welcome should you want to attack something else. Possible browser scrolling refinements: 1. Scroll by an integral number of labels. This is easy with text. For our synthesized tree, we would have to calculate the # of canvas pixels to scroll. However, if we switch to ttk.Treeview (#31552), it, like Text has a height in lines. So its yview method might scroll in line units, like text. 2. Only bind wheel event(s) used on system? Should test on 3 systems. Do all *nix other than tk for macOS use X-window buttons? Not a big deal. 3. Use bindtags to bind wheel events just once. Then unbind when shutdown? (Check if unbind elsewhere.) |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:59:19 | admin | set | github: 82083 |
| 2019年09月05日 04:13:49 | terry.reedy | set | status: open -> closed resolution: fixed messages: + msg351174 stage: patch review -> resolved |
| 2019年09月05日 01:58:25 | miss-islington | set | messages: + msg351172 |
| 2019年09月05日 01:53:50 | miss-islington | set | nosy:
+ miss-islington messages: + msg351171 |
| 2019年09月05日 01:35:24 | miss-islington | set | pull_requests: + pull_request15348 |
| 2019年09月05日 01:35:18 | miss-islington | set | stage: test needed -> patch review pull_requests: + pull_request15347 |
| 2019年09月05日 01:33:36 | terry.reedy | set | messages: + msg351166 |
| 2019年08月22日 18:22:03 | terry.reedy | set | messages: + msg350223 |
| 2019年08月22日 18:21:04 | terry.reedy | set | messages: + msg350222 |
| 2019年08月22日 17:49:46 | GeeTransit | set | messages: + msg350216 |
| 2019年08月22日 16:24:09 | GeeTransit | set | messages: + msg350210 |
| 2019年08月21日 21:41:50 | terry.reedy | set | messages:
+ msg350113 stage: patch review -> test needed |
| 2019年08月21日 21:16:56 | GeeTransit | set | messages: + msg350110 |
| 2019年08月21日 21:11:58 | GeeTransit | set | pull_requests: + pull_request15079 |
| 2019年08月21日 20:35:05 | GeeTransit | set | pull_requests: - pull_request15077 |
| 2019年08月21日 20:34:59 | GeeTransit | set | pull_requests: - pull_request15076 |
| 2019年08月21日 19:45:29 | GeeTransit | set | pull_requests: + pull_request15077 |
| 2019年08月21日 19:45:29 | GeeTransit | set | pull_requests: + pull_request15076 |
| 2019年08月21日 19:34:55 | GeeTransit | set | pull_requests: - pull_request15072 |
| 2019年08月21日 16:44:49 | GeeTransit | set | messages: + msg350095 |
| 2019年08月21日 15:19:32 | GeeTransit | set | keywords:
+ patch stage: needs patch -> patch review pull_requests: + pull_request15072 |
| 2019年08月21日 04:03:13 | terry.reedy | link | issue31461 dependencies |
| 2019年08月21日 04:03:08 | terry.reedy | set | messages: + msg350048 |
| 2019年08月21日 03:58:46 | terry.reedy | set | messages:
+ msg350047 stage: needs patch |
| 2019年08月21日 01:54:40 | GeeTransit | create | |