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: Remove close_when_done from colorizer close() #80333

Closed
csabella opened this issue Feb 28, 2019 · 4 comments
Closed

IDLE: Remove close_when_done from colorizer close() #80333

csabella opened this issue Feb 28, 2019 · 4 comments
Assignees
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 36152
Nosy @terryjreedy, @csabella, @miss-islington
PRs
  • bpo-36152: IDLE: Remove unused parameter from colorizer #12109
  • [3.7] bpo-36152: IDLE: Remove unused parameter from colorizer (GH-12109) #12119
  • 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-03-01.13:27:04.947>
    created_at = <Date 2019-02-28.23:51:49.343>
    labels = ['3.8', 'expert-IDLE', 'type-feature', '3.7']
    title = 'IDLE: Remove close_when_done from colorizer close()'
    updated_at = <Date 2019-03-01.14:52:47.862>
    user = 'https://github.com/csabella'

    bugs.python.org fields:

    activity = <Date 2019-03-01.14:52:47.862>
    actor = 'miss-islington'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2019-03-01.13:27:04.947>
    closer = 'cheryl.sabella'
    components = ['IDLE']
    creation = <Date 2019-02-28.23:51:49.343>
    creator = 'cheryl.sabella'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 36152
    keywords = ['patch']
    message_count = 4.0
    messages = ['336880', '336884', '336913', '336920']
    nosy_count = 3.0
    nosy_names = ['terry.reedy', 'cheryl.sabella', 'miss-islington']
    pr_nums = ['12109', '12119']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue36152'
    versions = ['Python 3.7', 'Python 3.8']

    @csabella
    Copy link
    Contributor Author

    Remove the unused close_when_done parameter from close() in colorizer.ColorDelegator().

    • The second parameter to close() is called close_when_done and it is expected to contain a toplevel widget that has a destroy() method.

    • Originally, the editor window had code that would send self.top (if colorizing was in process) as the value for this parameter:

                doh = colorizing and self.top
                self.color.close(doh) # Cancel colorization
    • This was changed via this commit (8ce8a78) in 2007 to instead be:

            self.color.close(False)
            self.color = None
      

    The value of False made it so the destroy code in colorizer wouldn't be run even though None or leaving the parameter off would have been more clear.

    In any case, this close_when_done hasn't been used since 2007.

    @csabella csabella self-assigned this Feb 28, 2019
    @csabella csabella added 3.7 (EOL) end of life 3.8 only security fixes topic-IDLE type-feature A feature request or enhancement labels Feb 28, 2019
    @csabella csabella assigned terryjreedy and unassigned csabella Mar 1, 2019
    @terryjreedy
    Copy link
    Member

    Thanks for digging up the history. I would have approved without, but feel better with. Using the colorizer close method to close a window seemed flakey to me, and it seems someone else thought the same long ago. I suspect the machinery was left to avoid breaking external uses. PEP-434 changed this avoidance being *necessary*. (I am sure I have seen a SO answer with percolator + colorizer, as in turtledemo.)

    IDLE has had a too many shutdown bugs and still has some. Not adding one was my main concern when reviewing.

    @csabella
    Copy link
    Contributor Author

    csabella commented Mar 1, 2019

    New changeset b9f0354 by Cheryl Sabella in branch 'master':
    bpo-36152: IDLE: Remove unused parameter from colorizer (GH-12109)
    b9f0354

    @csabella csabella closed this as completed Mar 1, 2019
    @miss-islington
    Copy link
    Contributor

    New changeset 70852b1 by Miss Islington (bot) in branch '3.7':
    bpo-36152: IDLE: Remove unused parameter from colorizer (GH-12109)
    70852b1

    @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
    3.7 (EOL) end of life 3.8 only security fixes topic-IDLE type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants