bpo-30781: IDLE - use ttk widgets in configdialog #2654

csabella · 2017-07-11T00:46:40Z

First set of commits for converting constants to string literals.

https://bugs.python.org/issue30781

mention-bot · 2017-07-11T00:46:44Z

@csabella, thanks for your PR! By analyzing the history of the files in this pull request, we identified @terryjreedy, @kbkaiser and @mlouielu to be potential reviewers.

mlouielu · 2017-07-11T03:44:53Z

Maybe I'm not on the date, does this a new policy to changed tkinter constant to string?

csabella · 2017-07-11T11:41:34Z

On the last file that was converted to use ttk widgets, I also changed the constants to literal strings at the same time because Terry prefers the literal strings. I was applying the same steps here.

csabella · 2017-07-11T23:44:51Z

The latest commit converts to ttk widgets. Some comments about the changes:

In order to show the color changes for the themes, I needed to use a Frame background. The only way to do this in ttk is with a style, but I tried to minimize the impact of adding Style since we don't want to convert to using that yet.
The Checkbutton no longer has an indicatoron option, so they look a lot different on the extensions tab.
I didn't convert Scale because the ttk version didn't have a tickinterval. I believe this is being changed anyway, so I left it as is for now.

terryjreedy · 2017-07-28T22:48:59Z

I am thinking that having NORMAL and DISABLED stand out is ok. Don't change though yet.
The change of from .config to subscripting created conflicts. We may introduce others before we get to this.

csabella · 2017-07-28T23:54:38Z

Yes, I had been keeping up with resolving the conflicts, but haven't done it in a few days, since the restructuring to the classes got the momentum. Once you're ready for this, I'll just do it all then.

terryjreedy

For the ttk issue, I want a minimal patch that only does what is needed to make the switch, or to be consistent about state ('disabled' instead of DISABLED with '!disabled', and 'normal'). I noted most but perhaps not all of these.

I believe everything other that the one note in the tests is needed. For configdialog, 80% is extraneous, so I presume it would be easier to start fresh.

I definitely want to switch to True, False, and widget['option'] = value, but in a separate style issue, at least after disposing of existing patches.

terryjreedy · 2017-08-25T04:53:08Z

Lib/idlelib/idle_test/test_configdialog.py

Make this 'disabled' to be consistent with '!disabled'.

terryjreedy · 2017-08-25T04:54:17Z

Lib/idlelib/configdialog.py

Leave these imports.

terryjreedy · 2017-08-25T05:17:07Z

Lib/idlelib/configdialog.py

terryjreedy · 2017-08-25T05:17:45Z

Lib/idlelib/configdialog.py

terryjreedy · 2017-08-25T05:18:01Z

Lib/idlelib/configdialog.py

Keep block.

terryjreedy · 2017-08-25T05:19:47Z

Lib/idlelib/configdialog.py

terryjreedy · 2017-08-25T05:19:59Z

Lib/idlelib/configdialog.py

terryjreedy · 2017-08-25T05:20:13Z

Lib/idlelib/configdialog.py

terryjreedy · 2017-08-25T05:20:23Z

Lib/idlelib/configdialog.py

terryjreedy · 2017-08-25T05:20:35Z

Lib/idlelib/configdialog.py

bedevere-bot · 2017-08-25T05:23:10Z

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 didn't expect the Spanish Inquisition!. 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.

From Terry. Please don't unless I ignore new commits for a couple of days. Just continue with decent commit messages.

csabella · 2017-08-25T14:30:58Z

I didn't expect the Spanish Inquisition!

bedevere-bot · 2017-08-25T14:31:00Z

Nobody expects the Spanish Inquisition!

@terryjreedy: please review the changes made to this pull request.

terryjreedy

Pending a 'human test', looks good.

In title of existing news blurb, Misc/NEWS.d/next/IDLE/2017-07-28-18-59-06.bpo-30781.ud5m18.rst, change 'Notebook' to 'widgets'. Add 'Patches by Terry Jan Reedy and Cheryl Sabella.

terryjreedy · 2017-08-25T18:23:53Z

Lib/idlelib/configdialog.py

+                         Notebook, Radiobutton, Scrollbar, Style)
 import tkinter.colorchooser as tkColorChooser
 import tkinter.font as tkFont
 import tkinter.messagebox as tkMessageBox


I want to change these in a subsequent 'style' patch, either with other style changes for this file or a separate patch for all files. Probably

from tkinter import colorchooser from tkinter import messagebox import tkinter.font as tkfont

I think tkfont may be necessary to not conflict with local font names. Will check with grep first.

bedevere-bot · 2017-08-25T18:45:55Z

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 didn't expect the Spanish Inquisition!. 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.

csabella · 2017-08-25T19:21:11Z

I didn't expect the Spanish Inquisition!

bedevere-bot · 2017-08-25T19:21:13Z

Nobody expects the Spanish Inquisition!

@terryjreedy: please review the changes made to this pull request.

Patch by Cheryl Sabella. (cherry picked from commit 7028e59)

) Patch by Cheryl Sabella. (cherry picked from commit 7028e59)

Patch by Cheryl Sabella.

the-knights-who-say-ni added the CLA signed label Jul 11, 2017

csabella force-pushed the bpo30781 branch 2 times, most recently from c008c06 to 3ed0cc5 Compare July 11, 2017 23:40

csabella force-pushed the bpo30781 branch 2 times, most recently from 5d794ae to 759370a Compare July 21, 2017 12:31

csabella force-pushed the bpo30781 branch from 6a8ba91 to 87d7ad0 Compare July 24, 2017 13:32

csabella force-pushed the bpo30781 branch 2 times, most recently from 8148041 to b607cc7 Compare August 24, 2017 23:18

terryjreedy requested changes Aug 25, 2017

View reviewed changes

bedevere-bot added the awaiting changes label Aug 25, 2017

csabella added 2 commits August 25, 2017 09:05

Rebase and update tests to pass for ttk.

b2f31aa

ttk only changes, including state

8015fa3

csabella force-pushed the bpo30781 branch from b607cc7 to 8015fa3 Compare August 25, 2017 14:19

bedevere-bot added awaiting change review and removed awaiting changes labels Aug 25, 2017

terryjreedy requested changes Aug 25, 2017

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting change review labels Aug 25, 2017

Update blurb.

82514b8

bedevere-bot added awaiting change review and removed awaiting changes labels Aug 25, 2017

terryjreedy approved these changes Aug 25, 2017

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Aug 25, 2017

terryjreedy changed the title ~~bpo-30781: IDLE: configdialog - switch to ttk widgets~~ bpo-30781: IDLE - use ttk widget in configdialog Aug 26, 2017

terryjreedy changed the title ~~bpo-30781: IDLE - use ttk widget in configdialog~~ bpo-30781: IDLE - use ttk widgets in configdialog Aug 26, 2017

terryjreedy merged commit 7028e59 into python:master Aug 26, 2017

terryjreedy pushed a commit to terryjreedy/cpython that referenced this pull request Aug 26, 2017

[3.6] bpo-30781: IDLE - use ttk widgets in configdialog (pythonGH-2654)

14d25b2

Patch by Cheryl Sabella. (cherry picked from commit 7028e59)

terryjreedy added a commit that referenced this pull request Aug 26, 2017

[3.6] bpo-30781: IDLE - use ttk widgets in configdialog (GH-2654) (#3214

7b55602

) Patch by Cheryl Sabella. (cherry picked from commit 7028e59)

csabella deleted the bpo30781 branch August 26, 2017 21:54

Mariatta removed the awaiting merge label Aug 29, 2017

GadgetSteve pushed a commit to GadgetSteve/cpython that referenced this pull request Sep 10, 2017

bpo-30781: IDLE - use ttk widgets in configdialog (python#2654)

ff7d3aa

Patch by Cheryl Sabella.

Uh oh!

bpo-30781: IDLE - use ttk widgets in configdialog #2654

bpo-30781: IDLE - use ttk widgets in configdialog #2654

Uh oh!

Conversation

csabella commented Jul 11, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mention-bot commented Jul 11, 2017

Uh oh!

mlouielu commented Jul 11, 2017

Uh oh!

csabella commented Jul 11, 2017

Uh oh!

csabella commented Jul 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

terryjreedy commented Jul 28, 2017

Uh oh!

csabella commented Jul 28, 2017

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Aug 25, 2017 • edited by terryjreedy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

csabella commented Aug 25, 2017

Uh oh!

bedevere-bot commented Aug 25, 2017

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Aug 25, 2017

Uh oh!

csabella commented Aug 25, 2017

Uh oh!

bedevere-bot commented Aug 25, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

csabella commented Jul 11, 2017 •

edited by bedevere-bot

Loading

csabella commented Jul 11, 2017 •

edited

Loading

bedevere-bot commented Aug 25, 2017 •

edited by terryjreedy

Loading