Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idle: require tk 8.5 and ttk widgets, and drop unneeded code. #68947

Closed
terryjreedy opened this issue Jul 30, 2015 · 31 comments
Closed

Idle: require tk 8.5 and ttk widgets, and drop unneeded code. #68947

terryjreedy opened this issue Jul 30, 2015 · 31 comments
Assignees
Labels
topic-IDLE type-feature A feature request or enhancement

Comments

@terryjreedy
Copy link
Member

BPO 24759
Nosy @terryjreedy, @ncoghlan, @ned-deily, @roseman, @serhiy-storchaka
Files
  • require85.diff
  • require85-v2.diff
  • require85-v3.diff
  • 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:

    assignee = 'https://github.com/terryjreedy'
    closed_at = <Date 2016-06-10.22:41:41.175>
    created_at = <Date 2015-07-30.22:18:35.616>
    labels = ['expert-IDLE', 'type-feature']
    title = 'Idle: require tk 8.5 and ttk widgets, and drop unneeded code.'
    updated_at = <Date 2019-03-23.04:15:36.380>
    user = 'https://github.com/terryjreedy'

    bugs.python.org fields:

    activity = <Date 2019-03-23.04:15:36.380>
    actor = 'terry.reedy'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2016-06-10.22:41:41.175>
    closer = 'terry.reedy'
    components = ['IDLE']
    creation = <Date 2015-07-30.22:18:35.616>
    creator = 'terry.reedy'
    dependencies = []
    files = ['43149', '43210', '43254']
    hgrepos = []
    issue_num = 24759
    keywords = ['patch']
    message_count = 31.0
    messages = ['247701', '247702', '247703', '247721', '247722', '247810', '247811', '247816', '247843', '247845', '247854', '247860', '247862', '247896', '247963', '247993', '248031', '266743', '266803', '267077', '267080', '267108', '267145', '267154', '267156', '267180', '267311', '267313', '267502', '268073', '268074']
    nosy_count = 7.0
    nosy_names = ['terry.reedy', 'ncoghlan', 'ned.deily', 'markroseman', 'python-dev', 'Al.Sweigart', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue24759'
    versions = ['Python 3.6']

    @terryjreedy
    Copy link
    Member Author

    PyShell currently has this code

    try:
    from tkinter import *
    except ImportError:
    print("** IDLE can't import Tkinter.\n"
    "Your Python may not be configured for Tk. **", file=sys.__stderr__)
    sys.exit(1)
    import tkinter.messagebox as tkMessageBox

    When Idle is started from an icon, there is no place for the error message to go, so it appears than nothing happens. But this is the best we can do without invoking system specific error message functions. (This, if possible, would be another issue.) The second import assumes that messagebox is available, which is should be without modification of the tkinter package since long ago. But I think we should guard against someone trying to start Idle on pre 8.5. The following seems to work nicely. This is a prerequisite for any ttk patches. Any comment before I apply?

    try:
    from tkinter import ttk
    except:
    root = Tk()
    root.withdraw()
    tkMessageBox.showerror("Fatal Idle Import Error",
    "Idle cannot import required module tkinter.ttk.\n"
    "Click OK to exit.",
    parent=root)
    sys.exit(1)

    @terryjreedy terryjreedy self-assigned this Jul 30, 2015
    @terryjreedy terryjreedy added the type-feature A feature request or enhancement label Jul 30, 2015
    @terryjreedy
    Copy link
    Member Author

    I tested by renaming installed 3.5 ttk, editing 3.5 PyShell with 3.4, and clicking 3.5 icon.

    @roseman
    Copy link
    Mannequin

    roseman mannequin commented Jul 30, 2015

    Sounds good. Only suggestion, given it's user facing, is to have the error message point them towards a solution. Off the top of my head, maybe something like "IDLE requires Python be configured to use Tk 8.5 or newer (you have Tk x.x)" with a title of "IDLE Cannot be Started"

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 31, 2015

    New changeset 8203fc75b3d2 by Terry Jan Reedy in branch '2.7':
    bpo-24759: Gracefull exit Idle if ttk import fails.
    https://hg.python.org/cpython/rev/8203fc75b3d2

    New changeset 13a8782a775e by Terry Jan Reedy in branch '3.4':
    bpo-24759: Gracefull exit Idle if ttk import fails.
    https://hg.python.org/cpython/rev/13a8782a775e

    @terryjreedy
    Copy link
    Member Author

    Improved message. Thanks. Should be good enough for the extremely few times it should ever be triggered.

    @serhiy-storchaka
    Copy link
    Member

    IMHO Tcl and Tk should be in title case (as Python or Django) or at least all in upper case (as APL or SDL).

    @serhiy-storchaka
    Copy link
    Member

    I don't see any mentions of ttk in IDLE source code except added check. Is ttk really required for IDLE?

    @terryjreedy
    Copy link
    Member Author

    Re-opened for tweak.

    This patch, to exit gracefully if ttk is not available, is the first step before using ttk in Idle. Mark, a long-time tk/ttk expert and site/book author, has volunteered to help upgrade Idle with ttk. Please see "ttk in idle" on Idle-Sig list for general discussion and bpo-24750 for the first substantive patches. The only reason I have not applied the ttk.Scrollbar patch, on that issue, is because Ned mentioned, on that issue, that the OS 8.5 python requires tcl/tk 8.4. I strongly feel that this should not stop the use of ttk, but I an first doing a few non-ttk patches, including Mark's patch for bpo-24745, before continuing.

    @terryjreedy terryjreedy reopened this Aug 1, 2015
    @terryjreedy
    Copy link
    Member Author

    Re-reading PEP-434, I was mistaken to apply this patch to 2.7 and 3.4 without further discussion. The PEP says "The PEP would apply to minor ..., but not necessarily to possible major re-writes such as switching to themed widgets ... ."

    Nick, should I post something on python-ideas or pydev, or continue here (moved from bpo-24750) about making the switch also in 2.7 and 3.4?

    I believe it would be best overall to at upgrade Idle 2.7, but I will not cry if we stop patching it now. Ditto for 3.4, which will soon get security fixes only anyway. I am ok with using ttk starting with 3.5.

    @terryjreedy terryjreedy changed the title Idle: require 8.5 / ttk Idle: require ttk (and tcl/tk 8.5) Aug 2, 2015
    @roseman
    Copy link
    Mannequin

    roseman mannequin commented Aug 2, 2015

    Got it re: 2.7/3.4. So the only stumbling block left to 8.5/ttk in 3.5 would be the current Mac build that is ok with Tcl/Tk 8.4...?

    @serhiy-storchaka
    Copy link
    Member

    Ttk widgets are partially compatible with Tk widgets, so in many cases it is possible to make partial upgrade without major rewriting. Some changes perhaps need major rewriting (using styles, changing layout), and I think this is what the PEP does not allow. However it is possible that all changes can be done without major rewriting, we don't know until try.

    @terryjreedy
    Copy link
    Member Author

    If 2.7 and 3.4 are left out of consideration, then, AFAIK, the only people necessarily affected are those with a PowerPC with OS 10.5 who upgrade to python3.5 and also want to run Idle. People with an Intel machine instead might be if there is no python3.5 that will run with the ActiveState 8.5 that Mark says works on Intel 10.5 machines. I wish I know how many people that is. I suspect a tiny fraction of Idle users and of people with such machines. But ...

    The compatibility approach will work for Scrollbars, but I am dubious about other widgets. For instance, tk Buttons have at least 31 options. ttk Buttons have 9 of the same + style (and class_, which I do not thing we would use), leaving 22 tk-only options.

    To write tk&ttk code *in one file*, I believe
    ...ttk.Button(parent, <options>, style='xyz')
    would have to be written (compactly) as something like
    b=...Button(parent, <common options>)
    b.config(**({'style':'xyz'} if ttk else { <tk style options>}))
    or (expansively, in 5 lines)
    b=...Button(parent, <common options>)
    if ttk:
    b['style'] = 'xyz'
    else:
    b.config(<tk style options in '=' format>})

    I consider this impractical. I am unwilling to write code that way, in part because it would have to be carefully tested both with and without ttk -- by hand. I cannot imagine anyone else doing so either. It also does not work for uses of ttk.Treeview, whose API is different from the multiple classes used in Path and Class (Module) browser). I believe the same is true for ttk.Notebook and the idlelib tab widget.

    What I already planned to do instead is copy existing dialog code to new files, possibly refactored, with PEP-8 filenames and internal names style. I already planned to leave the existing files in place for now, though as zombies, in case of any external code imports. With a little more work, it should be possible to optionally use either old or new files. Most of the work would be done with alternate bindings to menu items and accelerator keys, such as Find in Files and Alt-F3.

    It might be more acceptible to use ttk in 2.7 and 3.4 as an option rather than as a replacement. Though I would only commit a ttk version when confident that is works, leaving the option to switch back would add a safety factor. I changed the title and will revert the patch later.

    @terryjreedy terryjreedy changed the title Idle: require ttk (and tcl/tk 8.5) Idle: add ttk widgets as an option Aug 2, 2015
    @serhiy-storchaka
    Copy link
    Member

    Note also that committed patch doesn't work at all. "from tkinter import ttk" doesn't raise an exception with Tcl/Tk 8.4. See my patch in bpo-24750 that does working check.

    I consider impractical complex code for supporting 8.4 and ttk too. But in simplest cases this can be done easy.

    @terryjreedy
    Copy link
    Member Author

    I realize now that tkinter.ttk is (normally) present and will define Python classes even if the tk widgets needed for them to work are not present. More comments on the added module on bpo-24750

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 3, 2015

    New changeset 0511b1165bb6 by Terry Jan Reedy in branch '2.7':
    Issue bpo-24759: Revert 8203fc75b3d2.
    https://hg.python.org/cpython/rev/0511b1165bb6

    New changeset 06852194f541 by Terry Jan Reedy in branch '3.4':
    Issue bpo-24759: Revert 13a8782a775e.
    https://hg.python.org/cpython/rev/06852194f541

    New changeset 863e3cdbbabe by Terry Jan Reedy in branch '3.5':
    Issue bpo-24759: Merge with 3.4
    https://hg.python.org/cpython/rev/863e3cdbbabe

    New changeset 3bcb184b62f8 by Terry Jan Reedy in branch 'default':
    Issue bpo-24759: Merge with 3.5
    https://hg.python.org/cpython/rev/3bcb184b62f8

    @terryjreedy
    Copy link
    Member Author

    A second specific reason to make ttk optional is either they or the accompanying re-writing may (and probably will) break some extension that goes beyond the narrowly defined extension interface. For the present, a user using such an extension would be able to continue to do so, by turning use_ttk off. (I plan to add some DeprecationWarnings, either at startup or old tk module import, when use_ttk is possible but turned off.)

    The normal bugfix-only policy, and the Idle exemption, starts with each x.y.0b1 release. The point of excluding 'major re-writes such as switching to themed widgets' was to exclude changes in bugfix releases that prevent idle from running in the 'current' environment. In private email, Nick agreed with me that with ttk and any possible disablement made optional, it can be added to all current releases. He also suggested being on by default when possible.

    I decided not to put anything into 3.5.0. I intend to start pushing ttk patches perhaps next week after the release branch is split off and the main 3.5 branch is 3.5.1.

    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Aug 5, 2015

    IDLE's in an "interesting" place right now - it isn't showing people Tcl/Tk in its best light, so folks are likely to assume all Tcl/Tk apps necessarily look that way, and it's also using GUI idioms like separate shell and editor windows that don't reflect the conventions of modern IDEs.

    For 3.5.1+, I think there's no question that we want to show IDLE in the best possible light, and we want to try to do that by default. That means modernising it to use the best cross-platform features that Tcl/Tk has to offer (including ttk).

    However, Ned pointed out that the last PPC-supporting Mac OS X (10.5) has a Tcl/Tk version older than 8.5, and there's the general compatibility risk for breaking extensions with large refactorings, so retaining a "non-ttk mode" will be a valuable approach to the modernisation effort.

    @terryjreedy
    Copy link
    Member Author

    In msg265349, Ned stated that running IDLE with 8.4 should no longer be a requirement in 3.6. Hence the revised title and restriction to 3.6.

    Serhiy, I read the code in your bpo-24750 patch. Should it not be enough to first check tkinter.Tkversion >= 8.5 before trying to import ttk. And is testing ttk then needed? Is it possible for ttk to not work in 8.5/6)?

    Tkversion (or a refinement thereof) is already checked in colorizer, config, editor, macosx, and pyshell. With one check on startup, existing check can be eliminated, along with code only for older versions.

    @terryjreedy terryjreedy changed the title Idle: add ttk widgets as an option Idle: require tk 8.5 and ttk widgets, and drop unneeded code. May 31, 2016
    @serhiy-storchaka
    Copy link
    Member

    My patch in bpo-24750 makes sense only if we want support Tcl/Tk 8.4. I think that if require Tcl/Tk 8.5+, we can use Ttk directly and unconditionally. I don't think that it can be not working.

    @terryjreedy
    Copy link
    Member Author

    Given what Serhiy said, the core of this patch is the original patch with a test of TkVersion instead of importing ttk. Code that only ran for 8.4- is removed. Some minimal new tests are added, and I may add some more. None of the changes should depend on OS. I want to apply this in a few days as it is a prerequisite for ttk patches.

    @serhiy-storchaka
    Copy link
    Member

    Do you forgot to attach a patch?

    @terryjreedy
    Copy link
    Member Author

    Trying again. And yes, I would like a review. I don't think there is anything system specific, but some deletions required a minor rewrite.

    @serhiy-storchaka
    Copy link
    Member

    Did you try to run tests with Tk 8.4?

    @terryjreedy
    Copy link
    Member Author

    No, 3.4 is security fixes only and this is a 3.6 only issue.

    @serhiy-storchaka
    Copy link
    Member

    I meant Tk 8.4, not Python 3.4.

    Tests should be either passed or skipped, no errors raised.

    @terryjreedy
    Copy link
    Member Author

    I saw the review comment about adding lines.

    I do not have 8.4 either, but I get the point: the startup version check does not not guard the unittests. After test.text_idle imports unittest and tkinter (as tk) (line 6), I will add

    if tk.TkVersion < 8.5:
        raise unittest.SkipTest("IDLE requires tk 8.5 or later.")

    I will add a 'private API' and version-required notice to idlelib.idle_test.__init__. I will add version required to the notice already in idlelib.__init__.

    For similar reasons, the proposed interface module (bpo-27162) would also need an explicit check. (Note added there.)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 4, 2016

    New changeset e6560f018845 by Terry Jan Reedy in branch '2.7':
    Issue bpo-24759: Add 'private' notice for idlelib.idle_test.
    https://hg.python.org/cpython/rev/e6560f018845

    New changeset d75a25b3abe1 by Terry Jan Reedy in branch '3.5':
    Issue bpo-24759: Add 'private' notice for idlelib.idle_test.
    https://hg.python.org/cpython/rev/d75a25b3abe1

    @terryjreedy
    Copy link
    Member Author

    I believe require85-v2.diff adds everything specified in my last post.

    @terryjreedy
    Copy link
    Member Author

    Fixed whitespace and added comment.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 10, 2016

    New changeset 81927f86fa3a by Terry Jan Reedy in branch 'default':
    Issue bpo-24759: Add test for IDLE syntax colorizoer.
    https://hg.python.org/cpython/rev/81927f86fa3a

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 10, 2016

    New changeset 76f831e4b806 by Terry Jan Reedy in branch 'default':
    Issue bpo-24759: IDLE requires tk 8.5 and availability ttk widgets.
    https://hg.python.org/cpython/rev/76f831e4b806

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-IDLE type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants