gh-62142: IDLE - rework debugger UI #22947

roseman · 2020-10-24T20:42:03Z

updated to use ttk widgets throughout
simplified controls, changed go/step/etc. to image buttons
overall layout is a two-pane horizontal window (controls, stack, status on left, variables on right)
treeview widget used to replace custom scrolled-listbox stackviewer and custom canvas-based namespaceviewer
integrated locals/globals into a single treeview on right; can collapse/expand each independently instead of having checkboxes to control whether shown or not
removed scrolledlist.py (was previously used only by stackview)

Issue: IDLE Debugger: Improve GUI #62142

E-Paine · 2020-10-25T16:49:44Z

Is it possible to do without using preset image sizes and font? (if for some reason the default font is larger than normal, the difference is very obvious). Even if we cannot change the image and font sizes dynamically, I think we should avoid fixing the family.

- updated to use ttk widgets throughout - simplified controls, changed go/step/etc. to image buttons - overall layout is a two-pane horizontal window (controls, stack, status on left, variables on right) - treeview widget used to replace custom scrolled-listbox stackviewer and custom canvas-based namespaceviewer - integrated locals/globals into a single treeview on right; can collapse/expand each independently instead of having checkboxes to control whether shown or not

was being used by stack trace in debugger

- button captions now use TkIconFont (a bit bigger than before) - status line now uses default font, plus derives italic variation for errors

roseman · 2020-10-25T18:33:04Z

Oops, sorry about the committed merge (can you tell I'm not using git every day)? In any case, I've changed the hard-coded fonts as per Elisha's comment.

E-Paine

Massive visual improvement over the original! We have a few methods which don't do anything (add_varheader, mouse_moved_vars and leave_vars): I assume there's a plan for them but for now should we add a TODO to them?

Lib/idlelib/debugger.py

terryjreedy

I only reviewed the comments, not the whole patch.

I verified that scrolled list is only used here and can go if not used here.
What is the replacement?

Lib/idlelib/debugger.py

terryjreedy · 2020-10-31T23:40:22Z

Lib/idlelib/debugger.py

+                    func = match.group(1) + '.' + func
+            else:
+                func = selfval.__class__.__name__ + '.' + func
+        except Exception:


I strongly prefer that we not add more bare exceptions. Certainly not without a justification comment.

Lib/idlelib/debugger.py

terryjreedy · 2020-11-01T01:31:23Z

Lib/idlelib/debugger.py

+                # we've probably got the string representation of the
+                # object sent from the remote debugger, see if we can
+                # parse it into something useful
+                match = re.match('^<(?:.*)\.([^\.]*) object at 0x[0-9a-f]+>$',


Special chars are normal inside [] without \, so I think [^\.] and [^.] have the same effect (I prefer the latter) so that [^\\.] is needed to match 3 all 3 chars. Prefix r suppress strings escapes, but there are none, so I think it has no effect, though some prefer to use it for all REs. Outside the brackets, '\.' matches literal period, which is what want, whereas '\\.' matches multiple backslashes? I am not sure why later periods are excluded. CPython now prints addresses with A-F.

In any case, non-trivial REs should be defined at the top of the file so that they can be tested.

terryjreedy · 2020-11-01T01:34:48Z

Lib/idlelib/debugger.py

+                # we've probably got the string representation of the
+                # object sent from the remote debugger, see if we can
+                # parse it into something useful
+                match = re.match('^<(?:.*)\.([^\.]*) object at 0x[0-9a-f]+>$',


I suspect this is correct. See previous comment about testing REs

Lib/idlelib/debugger.py

bedevere-bot · 2020-11-01T01:38:47Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

terryjreedy · 2020-11-02T03:17:37Z

All REs with '' should be prefixed with 'r' because uses like . and \d not recognized by standard string processing are deprecated and will become errors in the future.

Co-authored-by: Terry Jan Reedy <[email protected]> Co-authored-by: E-Paine <[email protected]>

Lib/idlelib/debugger.py

roseman · 2020-11-04T16:56:29Z

cleaned up the code and documentation in add_stackframee... not a shock but it was actually not doing anything as is because the 're' module hadn't been imported, which the bare exception oh-so-helpfully hid.. 😬

E-Paine · 2020-11-04T17:01:04Z

Just a general thing, is there a way to make the panedwindow more obvious? (when I first looked at the new design, I mistook it for padding until I looked at the code)

terryjreedy

Follow PEP-8 in comments and my comment on clarifying debugger separation.

The IDLE Doc says of debugger "This feature is still incomplete and somewhat experimental. " Would you consider this still true after this patch?

Lib/idlelib/debugger.py

E-Paine · 2020-11-04T22:13:53Z

Ubuntu failure related to issue 42142 (I have noted this there)

terryjreedy · 2020-11-04T22:44:24Z

@E-Paine Thank you. GH will not let me download the failure log.

…idth

Co-authored-by: Terry Jan Reedy <[email protected]>

E-Paine · 2020-11-17T12:10:50Z

I believe all review comments have been addressed except for the lazy/greedy regex one
Does anyone have a high-dpi monitor to test this patch on? (I am concerned that the images will be too small so very hard to see on such a monitor)

roseman · 2020-12-06T17:55:55Z

related to issue #14111... i note that the 'stop' button in the in-progress debugger ui is actually doing 'quit' (and doesn't in fact work when in the middle of a busy loop).

i think it would make sense to add a 'pause' button beside go, which can be used to pause the running program, allowing it to be continued. i'm wondering if the stop button should be renamed or icon changed to make it more clear that it will terminate the running program altogether.

should probably add tool tips on the debugger controls as well

github-actions · 2021-01-06T00:41:39Z

This PR is stale because it has been open for 30 days with no activity.

roseman requested a review from terryjreedy as a code owner October 24, 2020 20:42

the-knights-who-say-ni added the CLA signed label Oct 24, 2020

bedevere-bot added the awaiting review label Oct 24, 2020

roseman added 3 commits October 25, 2020 10:39

scrolledlist no longer needed

3cabc0a

was being used by stack trace in debugger

adjust fonts to use standard named fonts

a8b8da4

- button captions now use TkIconFont (a bit bigger than before) - status line now uses default font, plus derives italic variation for errors

roseman requested review from markshannon, methane and rhettinger as code owners October 25, 2020 17:44

roseman marked this pull request as draft October 25, 2020 18:15

roseman force-pushed the debugger-new-ui-17942 branch from 06a7185 to a8b8da4 Compare October 25, 2020 18:26

roseman marked this pull request as ready for review October 25, 2020 18:33

methane removed their request for review October 26, 2020 06:54

markshannon removed their request for review October 27, 2020 18:58

E-Paine reviewed Oct 31, 2020

View reviewed changes

terryjreedy requested changes Nov 1, 2020

View reviewed changes

bedevere-bot removed the awaiting review label Nov 1, 2020

bedevere-bot added the awaiting changes label Nov 1, 2020

roseman and others added 3 commits November 2, 2020 07:39

Apply suggestions from code review

7787255

Co-authored-by: Terry Jan Reedy <[email protected]> Co-authored-by: E-Paine <[email protected]>

apply suggestions from code review

37399ed

apply suggestions from code review

041d552

E-Paine reviewed Nov 2, 2020

View reviewed changes

Lib/idlelib/debugger.py Show resolved Hide resolved

clean up based on review comments - re, exception

448527e

explicit tkinter imports as per review comments

a057032

terryjreedy requested changes Nov 4, 2020

View reviewed changes

Lib/idlelib/debugger.py Outdated Show resolved Hide resolved

Lib/idlelib/debugger.py Outdated Show resolved Hide resolved

Lib/idlelib/debugger.py Outdated Show resolved Hide resolved

roseman and others added 4 commits November 15, 2020 14:25

status line always goes below toolbar buttons, regardless of window w…

4304969

…idth

Apply suggestions from code review (comments)

812768f

Co-authored-by: Terry Jan Reedy <[email protected]>

missing import

72740a3

tweak comments as per review suggestions

f9f9331

github-actions bot added the stale Stale PR or inactive for long period of time. label Jan 6, 2021

rhettinger removed their request for review May 3, 2022 06:15

ezio-melotti removed the CLA signed label Jul 13, 2022

github-actions bot removed the stale Stale PR or inactive for long period of time. label Jul 30, 2022

terryjreedy added the topic-IDLE label Nov 2, 2022

terryjreedy self-assigned this Nov 2, 2022

terryjreedy changed the title ~~bpo-17942: major rework of debugger UI~~ gh-62142: IDLE - major rework of debugger UI Nov 2, 2022

terryjreedy changed the title ~~gh-62142: IDLE - major rework of debugger UI~~ gh-62142: IDLE - rework debugger UI Nov 2, 2022

terryjreedy mentioned this pull request Aug 6, 2024

IDLE Debugger: Improve GUI #62142

Open

Uh oh!

gh-62142: IDLE - rework debugger UI #22947

Are you sure you want to change the base?

gh-62142: IDLE - rework debugger UI #22947

Uh oh!

Conversation

roseman commented Oct 24, 2020 • edited by terryjreedy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

E-Paine commented Oct 25, 2020

Uh oh!

roseman commented Oct 25, 2020

Uh oh!

E-Paine left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

terryjreedy Oct 31, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

terryjreedy Nov 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

terryjreedy Nov 1, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bedevere-bot commented Nov 1, 2020

Uh oh!

terryjreedy commented Nov 2, 2020

Uh oh!

Uh oh!

roseman commented Nov 4, 2020

Uh oh!

E-Paine commented Nov 4, 2020

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

E-Paine commented Nov 4, 2020

Uh oh!

terryjreedy commented Nov 4, 2020

Uh oh!

E-Paine commented Nov 17, 2020

Uh oh!

roseman commented Dec 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

roseman commented Oct 24, 2020 •

edited by terryjreedy

Loading

terryjreedy Nov 1, 2020 •

edited

Loading

roseman commented Dec 6, 2020 •

edited

Loading