Repository navigation
Idle: improve stack viewer #68978
Description
Activity
Debug => Stack Viewer (no hot key) could become more useful, aside from bpo-23544, freezing Idle when Debugger active.
- Don't include idlelib.run.runcode.
- Don't duplicate globals under each function. Once for the module is enough.
- Remove +Locals under each function and instead display locals when expanding Clicking a function should display locals without having to click anything else.
- Add a button to expand all locals?
- Change title to something better than 'Idle'. Perhaps 'Idle Shell Traceback'?
5b. Add Python version to end of , as with Shell window.
This will be more than one patch. Some should be easy.
canibanoglu commented
on Aug 17, 2015 canibanoglumannequinMannequinHello,
I'm very interested in tackling this issue but this will be my first contribution to Python and I'm quite nervous. I have gone over the developer guide and FAQ but I would appreciate if you could let me know if I should approach this in a certain way.
Welcome. I was hoping a new contributor would see the 'easy' tag. Nervous is okay as long as it leads to heighten carefulness without paralysis. I am still nervous hitting the [Commit] button.
-
Important. Sign the contributor agreement.
https://www.python.org/psf/contrib/
https://www.python.org/psf/contrib/contrib-form/
I will not look at a patch until you say you have signed. I will not commit until the '*' appears on your name (perhaps a week). -
Once you sign, I will give you at least a week to produce something before I work on this by myself. Patch needs to apply to 3.4, but StackViewer.py should be identical for 3.4/5/6.
-
Post a patch as soon as you do a complete chunk (any of the numbered items). Start with either 1. or 2. and switch if you get stuck.
Does above answer your question?
-
canibanoglu commented
on Aug 17, 2015 canibanoglumannequinMannequinIt does indeed, thank you very much! I have signed the Contributor Agreement and will start working on this right away.
canibanoglu commented
on Aug 18, 2015 canibanoglumannequinMannequinHello again,
I have spent some time going over the code and how the PyShell brings all of this together and have formed some loose opinions on how to solve these issues but I wanted to get a much more informed opinion.
I will provide a patch for the first one first, so right now, all my questions are related to that one.
Basically, I have come up with three different places I could patch in StackViewer.py to remove "idlelib.run.runcode" from the stack viewer.
First option is to patch the constructor of "StackTreeItem" so that if "idlelib.run.runcode" is in the traceback, it never gets pushed to the stack list. I don't like this approach very much because I would essentially be removing data from the stack.
Second option is to patch the "GetSubList" method so that the iterated variable "info" is checked before constructing a new "FrameTreeItem". If "idlelib.run.runcode" is matched, we would just continue to the next value in the list. I think this would lead to repetitive code though. We would rewrite some of the "GetText" method of "FrameTreeItem" and then make a string comparison to identify the entry to be removed.
Third option is to call the "GetText" method after each created "FrameTreeItem" and make the string comparison then. This would add a lot of method calls if the stack is long. On the other hand, this approach doesn't repeat any code.
It is highly possible that I have missed something very simple and elegant :) I would very much appreciate your input. I can make upload a patch once I get your approval. Which approach would be better here?
Something else that I would like to ask is the preparation of the patch file. Should switch to the 3.4 branch and then create the patch or can I do it at the dev branch? How would I check if the patch applies to 3.4/3.5/3.6 (apart from applying them manually and testing if it has the desired outcome)?
I'm sorry for the wall of text, I'm nervous and I don't want to mess something up. :)
Thanks a lot for your assistance.
StackViewer.py seem to be identical in 3.4, 3.5, and 3.6, so a patch against any applies to all. The diff with 2.7 is
F:\Python\dev\36>hg diff -r 4884af6d3e30 f:/python/dev/36/Lib/idlelib/StackViewer.py @@ -2,7 +2,7 @@ import sys import linecache import re -import Tkinter as tk +import tkinter as tk
from idlelib.TreeWidget import TreeNode, TreeItem, ScrolledCanvas from idlelib.ObjectBrowser import ObjectTreeItem, make_objecttreeitem @@ -10,7 +10,7 @@
def StackBrowser(root, flist=None, tb=None, top=None): if top is None: - from Tkinter import Toplevel + from tkinter import Toplevel top = Toplevel(root) sc = ScrolledCanvas(top, bg="white", highlightthickness=0) sc.frame.pack(expand=1, fill="both") @@ -109,7 +109,7 @@ return len(self.object) > 0def keys(self): - return self.object.keys() + return list(self.object.keys()) def GetSubList(self): sublist = []
The second and third differences are due to unneeded code; I will remove them after I submit this message, so update your repository after I do that and then patch.
---
Your research is very helpful. The purpose of 1. *is* to remove data ;-) -- data that is only present because of Idle. The purpose of StackTreeItem.get_stack is to removed unneeded data while converting a linked list to a regular list. Note that the loop drops 2 of 4 fields. I do not know if
if tb and tb.tb_frame is None:
tb = tb.tb_next
actually removes anything, but after this, I believe that the idlelib.run.runcode node (or idlelib.PyShell.runcode node, when Idle is started with -n) is at the top of the linked list. So I believe adding
tb = tb.tb_next
after the above will do what we want. Try it, and if it works, move on.---
6 (modeled after 3). Modules only have +Globals. Remove +Globals under each module and instead display globals when expanding. (There will only be a module other than __main__ when there is an error in another module being imported. We should then improve the lines to read
+ Globals for module __main__ ...
+ Locals for function xyz...python-dev commented
on Aug 18, 2015 python-devmannequinMannequinNew changeset 9ad330a9d785 by Terry Jan Reedy in branch '2.7':
Issue bpo-24790: Remove extraneous code (which also create 2 & 3 conflicts).
https://hg.python.org/cpython/rev/9ad330a9d785New changeset 010264c9ceae by Terry Jan Reedy in branch '3.4':
Issue bpo-24790: Remove extraneous code (which also create 2 & 3 conflicts).
https://hg.python.org/cpython/rev/010264c9ceae-
Sorts global and local names (as with dir()).
-
Don't expand values with their class attributes, at least not for builtins. Example
+ <dir icon> __doc__ = None
Clicking + gives all the attributes of type(None), each with its own +. It does not do this for ints and strings.
-
jimjjewett commented
on Aug 18, 2015 jimjjewettmannequinMannequinTerry: Was removing the public attribute keys OK?
6 remaining items
I just realized that there is a slight but in the patch so far. When a non-empty program runs, it necessarily executes at least one statement in the main module. It will only execute anything else when there is a call. Therefore, the stack trace always begins in the main module and that line can be used to display globals, making the globals item under functions redundant.
However, if there is a call to a function in another module, there need not be a module level statement. By removing the globals items for functions in other modules, the current patch as is will not allow viewing of globals in other modules. A possible solution is to add an expandable module item every time the module changes.
Mark and I would like to combine the StackViewer and Debugger UIs. Essentially, a revised stackviewer pane would eventually become part of the debugger. The Debugger avoids the intermediate +locals and +globals items of StackViewer by expanding both when an item is clicked. However, the expansion is not added to the tree, but to the one locals box and the one (optional) globals box, both separate from and below the tree.
An alternate solution to the globals problem is to expand locals under a function when clicked on, as currently done with stackviewer, and expand globals to a globals box below the tree, as currently done with stackviewer. I do not see much need to see the globals of different functions simultaneously (one can switch, as one must do not with Debugger). I see more reason to see multiple locals of at the same time, and it also make sense to put locals right under each function.
Debugger already does item 8.
We should take a look as the stackviewer part of the debugger ui.
canibanoglu commented
on Aug 25, 2015 canibanoglumannequinMannequinFirst of all, sorry for the late reply.
I did try the solution you have proposed which removes the need to click to expand the items under globals/locals but as I understand it, you have something more in mind for the whole stackviewer UI. Should I still submit a patch for this?
As far as the bug is concerned I will look into that next.
Please post a 'checkpoint' patch with all changes so far, even though it has the 'other globals' problem.
Then look at the UI parts of Debugger.py. It has a Stackviewer class that subclasses ScrolledLIst instead of TreeWidget and a Namespaceviewer class currently used for both locals and globals.
The Namespace class is a frame containing a label (a labeled frame would work as well), a scrollbar, and a canvas containing a frame. The inner frame has a two-column grid: labels with names and entry boxes with values. Currently, the entry boxes do nothing other than make values look different from names, as editing the entry does nothing. (Ignored entries are a bad UI design.) Perhaps there was once an intention to propagate changes back to the running program. This would not apply to post-mortem stack viewing.
Starting with the checkpoint patch, I would like to modify Stackviewer to display globals for any item clicked in a separate frame, like Debugger. This would removing the bug in the checkpoint patch and result in a patch that I might commit. It would also be a step toward merging (in a new issue).
One possibility is a tree in a new frame. Another is to import and reuse Debugger.Namespaceviewer. The issue is whether users should be able to view the attributes of value objects (as in Stackviewer) or not (as in Debugger). I previously suggested not, because it makes the interface look busy, but I just rediscovered a note based on debugger experience suggesting that it might be a good idea. Lets start with a second tree.
When waiting for me to respond, you could look at bpo-17942, about improving the command button part of debugger.
python-dev commented
on Aug 27, 2015 python-devmannequinMannequinNew changeset 14ac7deba91f by Terry Jan Reedy in branch '2.7':
Issue bpo-24790: Restore unused function.
https://hg.python.org/cpython/rev/14ac7deba91fNew changeset eec5e81b38d1 by Terry Jan Reedy in branch '3.4':
Issue bpo-24790: Restore unused function.
https://hg.python.org/cpython/rev/eec5e81b38d1python-dev commented
on Aug 27, 2015 python-devmannequinMannequinNew changeset 514f5d610175 by Terry Jan Reedy in branch '2.7':
Issue bpo-24790: correct typo noticed by Eric Smith
https://hg.python.org/cpython/rev/514f5d610175New changeset 213ae5626493 by Terry Jan Reedy in branch '3.4':
Issue bpo-24790: correct typo noticed by Eric Smith
https://hg.python.org/cpython/rev/213ae5626493Deleted .keys in 8.6.
- As of now, values are not named, whereas I believe they are in debugger. IE given def f(a,b,c): 1/0; f(1,2,3), viewer prints unlaabelled 1, 2, 3 on separate lines.
Also, the representation of a value may not fit on a line. I believe a current debugger patch addresses this issue. Clearly, there should be one function for displaying locals and globals. And in debugger, one should be able to display them for previous frames.
Metadata
Metadata
Assignees
Projects
- Status
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:
bugs.python.org fields: