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

Jump to bottom

IDLE: Modernize config_key module #79779

Closed
csabella opened this issue Dec 27, 2018 · 18 comments
Closed

IDLE: Modernize config_key module #79779

csabella opened this issue Dec 27, 2018 · 18 comments
Assignees
@terryjreedy
Labels
3.7 (EOL) end of life 3.8 only security fixes topic-IDLE type-feature A feature request or enhancement

Comments

@csabella
Copy link
Contributor

BPO 35598
Nosy @terryjreedy, @csabella, @miss-islington
PRs
  • bpo-35598: IDLE: Update config_key.py with PEP8 names #11330
  • [3.7] bpo-35598: IDLE: Update config_key.py with PEP8 names (GH-11330) #11336
  • bpo-35598: IDLE: Increase test coverage for config_key.py #11360
  • [3.7] bpo-35598: IDLE: Increase test coverage for config_key.py (GH-11360) #11362
  • bpo-35598: IDLE: Switch to ttk widgets and update imports #11365
  • [3.7] bpo-35598: IDLE: Switch config_key dialog to ttk widgets (GH-11365) #11370
  • bpo-35598: IDLE: Refactor globals in config_key.py #11377
  • bpo-35598: IDLE: Refactor globals in config_key.py #11392
  • [3.7] bpo-35598: IDLE - Globalize some config_key objects (GH-11392) #11393
  • gh-79856: IDLE - separate config_key window and frame #11427
    		•

    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 2019-01-07.04:01:19.826>
created_at = <Date 2018-12-27.20:27:57.070>
labels = ['3.8', 'expert-IDLE', 'type-feature', '3.7']
title = 'IDLE: Modernize config_key module'
updated_at = <Date 2019-01-07.04:01:19.825>
user = 'https://github.com/csabella'

    bugs.python.org fields:

    activity = <Date 2019-01-07.04:01:19.825>
actor = 'terry.reedy'
assignee = 'terry.reedy'
closed = True
closed_date = <Date 2019-01-07.04:01:19.826>
closer = 'terry.reedy'
components = ['IDLE']
creation = <Date 2018-12-27.20:27:57.070>
creator = 'cheryl.sabella'
dependencies = []
files = []
hgrepos = []
issue_num = 35598
keywords = ['patch', 'patch', 'patch']
message_count = 18.0
messages = ['332614', '332617', '332634', '332635', '332638', '332640', '332738', '332742', '332744', '332758', '332759', '332760', '332821', '332829', '332830', '332836', '332953', '333136']
nosy_count = 3.0
nosy_names = ['terry.reedy', 'cheryl.sabella', 'miss-islington']
pr_nums = ['11330', '11336', '11360', '11362', '11365', '11370', '11377', '11392', '11393', '11427']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue35598'
versions = ['Python 3.7', 'Python 3.8']
    @csabella
    Copy link
    Contributor Author

    • Apply PEP-8 naming convention.
    • Add additional tests to get coverage (close?) to 100%.
    • Update to more meaningful names.
    • Switch to ttk widgets and revise imports.
    • Split toplevel class into a window class and frame class(es).

    @csabella csabella added the 3.8 only security fixes label Dec 27, 2018
    @csabella csabella added topic-IDLE type-feature A feature request or enhancement labels Dec 27, 2018
    @csabella
    Copy link
    Contributor Author

    PR11330 is for the PEP-8 naming conventions.

    @terryjreedy
    Copy link
    Member

    I am going to merge on the presumption that you will follow through at least with more tests. Currently missing other than mac-code, according to coverage output: switch from 'advanced to basic and calls to final_key_selected, build_key_string, get_modifiers, and translate_key. The last should become a standalone function.

    I believe I have encountered a bug. Change a key binding. Cancel. Re-open config dialog. Try to change back. It says original binding is in use -- which it is if one closes IDLE and reopens, or opens a different instance. It seems that cancel is not properly undoing the temporary change.

    @terryjreedy terryjreedy added the 3.7 (EOL) end of life label Dec 28, 2018
    @terryjreedy
    Copy link
    Member

    If you submit further PRs for this issue, the blurb will need to be changed as blurbs are for issues, not PRs.

    @terryjreedy
    Copy link
    Member

    New changeset 55698cc by Terry Jan Reedy (Cheryl Sabella) in branch 'master':
    bpo-35598: IDLE: Update config_key.py with PEP-8 names (GH-11330)
    55698cc

    @miss-islington
    Copy link
    Contributor

    New changeset 4c7f34f by Miss Islington (bot) in branch '3.7':
    bpo-35598: IDLE: Update config_key.py with PEP-8 names (GH-11330)
    4c7f34f

    @csabella
    Copy link
    Contributor Author

    PR11360 adds tests to increase coverage. There isn't any refactor (moving translate_key) as part of this. I also didn't add any GUI-related tests against the buttons, entry, or listbox, except for the cancel key (in case that helps with the new bug reported).

    @terryjreedy
    Copy link
    Member

    New changeset b0a6196 by Terry Jan Reedy (Cheryl Sabella) in branch 'master':
    bpo-35598: IDLE: Increase test coverage for config_key.py (bpo-11360)
    b0a6196

    @miss-islington
    Copy link
    Contributor

    New changeset 34aadec by Miss Islington (bot) in branch '3.7':
    bpo-35598: IDLE: Increase test coverage for config_key.py (GH-11360)
    34aadec

    @csabella
    Copy link
    Contributor Author

    PR11365 revises the imports and switches to ttk widgets.

    @terryjreedy
    Copy link
    Member

    New changeset 4bd79c3 by Terry Jan Reedy (Cheryl Sabella) in branch 'master':
    bpo-35598: IDLE: Switch config_key dialog to ttk widgets (GH-11365)
    4bd79c3

    @miss-islington
    Copy link
    Contributor

    New changeset d2694d4 by Miss Islington (bot) in branch '3.7':
    bpo-35598: IDLE: Switch config_key dialog to ttk widgets (GH-11365)
    d2694d4

    @csabella
    Copy link
    Contributor Author

    PR11392 is the first refactor. It moves translate_key to the module level and also moves the definitions of the key tuples to the module level since they are used in more than one place (and they don't change).

    As a side note, I'll do the refactoring over several PRs to try to make it manageable to review.

    (PR11377 was the first version of this, but somehow I really messed it and couldn't get it back.)

    @terryjreedy
    Copy link
    Member

    New changeset b4ea8bb by Terry Jan Reedy (Cheryl Sabella) in branch 'master':
    bpo-35598: IDLE - Globalize some config_key objects (GH-11392)
    b4ea8bb

    @miss-islington
    Copy link
    Contributor

    New changeset 74e4648 by Miss Islington (bot) in branch '3.7':
    bpo-35598: IDLE - Globalize some config_key objects (GH-11392)
    74e4648

    @terryjreedy
    Copy link
    Member

    This issue looks complete to me unless there is something simple not previously mentioned.

    Fixing cancel would be a separate issue.

    Separating window and frame needs separate discussion on a separate issue, and is not a priority now. Model popups are different from independent listed windows, so I an not sure what we will want to do for a future tabbed window.

    A more immediate concern to me is finishing ttk conversion, including using ttk frames for ttk widgets. (See bpo-33987 for why.)

    @csabella
    Copy link
    Contributor Author

    csabella commented Jan 4, 2019

    Terry,

    I just saw your note about waiting to split this into a Window and Frame class, which was after I had already gotten the PR ready. I've been mostly offline for the past few days, so I had been working on those changes locally with the intent of pushing them once I was back online. I understand if it's not a priority to review. My main goal in splitting them was more for readability rather than for being able to add it to a Tabbed window. As a follow up to this refactor, I had hoped to split the Basic and Advanced frames into their own tabs, mostly to clean up the create_widgets and to organize the supporting functions.

    Anyway, PR11427 refactors the main frame from the window.

    @terryjreedy
    Copy link
    Member

    I moved PR 11427 to new issue bpo-35675.

    @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
    Assignees

    @terryjreedy terryjreedy

    Labels
    3.7 (EOL) end of life 3.8 only security fixes topic-IDLE type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @terryjreedy @csabella @miss-islington