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.

Sign up for GitHub

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

Derby: Convert the _tkinter module to use Argument Clinic #64367

Closed
serhiy-storchaka opened this issue Jan 7, 2014 · 18 comments
Closed

Derby: Convert the _tkinter module to use Argument Clinic #64367

serhiy-storchaka opened this issue Jan 7, 2014 · 18 comments
Assignees
@serhiy-storchaka
Labels
extension-modules C modules in the Modules dir topic-argument-clinic topic-tkinter type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 20168
Nosy @larryhastings, @jkloth, @serhiy-storchaka, @zooba
Dependencies
  • bpo-20196: Argument Clinic generates invalid code for optional parameter
    		•
    Files
  • tkinter_clinic.patch
  • tkinter_clinic_2.patch
  • tkinter_clinic_3.patch
  • tkinter_clinic_4.patch
  • larry.fix.tkinter.build.on.windows.diff.1.txt
    		•

    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/serhiy-storchaka'
closed_at = <Date 2015-05-04.12:47:16.563>
created_at = <Date 2014-01-07.21:03:59.546>
labels = ['extension-modules', 'type-feature', 'expert-tkinter', 'expert-argument-clinic']
title = 'Derby: Convert the _tkinter module to use Argument Clinic'
updated_at = <Date 2015-05-04.12:47:16.562>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2015-05-04.12:47:16.562>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-05-04.12:47:16.563>
closer = 'serhiy.storchaka'
components = ['Extension Modules', 'Tkinter', 'Argument Clinic']
creation = <Date 2014-01-07.21:03:59.546>
creator = 'serhiy.storchaka'
dependencies = ['20196']
files = ['33528', '34533', '35437', '39091', '39282']
hgrepos = []
issue_num = 20168
keywords = ['patch']
message_count = 18.0
messages = ['207600', '207604', '207605', '207609', '207699', '207706', '207716', '208398', '214259', '219507', '241357', '242471', '242478', '242503', '242507', '242528', '242530', '242533']
nosy_count = 6.0
nosy_names = ['larry', 'jkloth', 'gpolo', 'python-dev', 'serhiy.storchaka', 'steve.dower']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20168'
versions = ['Python 3.5']
    @serhiy-storchaka
    Copy link
    Member Author

    In progress. Actually many functions are not very suitable for Argument Clinic.

    @serhiy-storchaka serhiy-storchaka added topic-tkinter extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Jan 7, 2014
    @serhiy-storchaka serhiy-storchaka self-assigned this Jan 7, 2014
    @larryhastings
    Copy link
    Contributor

    What functions, and what makes them unsuitable?

    @serhiy-storchaka
    Copy link
    Member Author

    call() just converts all args to Tcl list. Is Argument Clinic supports *args
    and **kwargs? I'm not sure there is a benefit with using Argument Clinic here.

    splitlist() and split() first call PyArg_ParseTuple with one argument, check
    the input, and then may call PyArg_ParseTuple with other argument. They need
    large refactoring to use Argument Clinic.

    getint(), getdouble(), getboolean() first manually unpack arguments tuple, and
    then may call PyArg_ParseTuple. Same as above.

    setvar()/globalsetvar(), getvar()/globalgetvar(), unsetvar()/globalunsetvar()
    shares common code parametrized by flags. I think this is resolvable. But
    worse, they can delegates their execution to other thread and should save all
    arguments in events queue. And this looks absolutely unsuitable for Argument
    Clinic.

    @larryhastings
    Copy link
    Contributor

    (It would have made it easier on me if you'd used the C function names, instead of the names in the module.)

    I agree. In retrospect, it's not surprising that things in _tkinter aren't suitable, as it appears to be a shunt for calling things in Tcl.

    For what it's worth, Argument Clinic is only intended for functions that use PyArg_ParseTuple or PyArg_ParseTupleOrKeywords to parse their arguments. So something like call is and will forever be unsuitable.

    (And reading _tkinter.c this has reminded me why I don't like Tcl ;-)

    @larryhastings
    Copy link
    Contributor

    So do you think there are any entry points worth converting in _tkinter, or should we close this issue?

    @serhiy-storchaka
    Copy link
    Member Author

    You misunderstood me. I already have 1018-lines patch which converts about 24
    functions and methods to Argument Clinic church. And I'm working on left
    functions. Do you want to get unfinished patch?

    @larryhastings
    Copy link
    Contributor

    Oh, okay. That sounds fine, and I'm not in a hurry. Please post the patch whenever you're ready, thanks!

    @serhiy-storchaka
    Copy link
    Member Author

    Here is a patch.

    @serhiy-storchaka
    Copy link
    Member Author

    Here is updated patch.

    @serhiy-storchaka
    Copy link
    Member Author

    Synchronized with tip.

    @serhiy-storchaka
    Copy link
    Member Author

    Updated to the tip. Converted methods getint, getdouble, getboolean, splitlist, split.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 3, 2015

    New changeset b22ced894d51 by Serhiy Storchaka in branch 'default':
    Issue bpo-20168: Converted the _tkinter module to Argument Clinic.
    https://hg.python.org/cpython/rev/b22ced894d51

    @serhiy-storchaka
    Copy link
    Member Author

    wantobjects() was not converted due to a bug in Argument Clinic (bpo-24051).

    @zooba
    Copy link
    Member

    zooba commented May 3, 2015

    This broke Windows builds because of unnecessary "static" qualifiers on the forward declarations at lines 2685 and 3006 (as discussed on bpo-20323).

    Removing "static" from these lines fixes the build.

    @zooba zooba reopened this May 3, 2015
    @larryhastings
    Copy link
    Contributor

    As mentioned on bpo-20148, removing the "static" from the forward declarations breaks it on GCC. (And, of the two, I think GCC is the one being reasonable here.)

    Attached is a patch that should fix the build for tkinter.

    @zooba
    Copy link
    Member

    zooba commented May 3, 2015

    LGTM

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 3, 2015

    New changeset 7a76c462c7f6 by Larry Hastings in branch 'default':
    Fix Windows build breakage from checkins on Issues bpo-20148 and bpo-20168.
    https://hg.python.org/cpython/rev/7a76c462c7f6

    @larryhastings
    Copy link
    Contributor

    Steve, please close this issue when you've confirmed it's now building correctly on Windows.

    @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

    @serhiy-storchaka serhiy-storchaka

    Labels
    extension-modules C modules in the Modules dir topic-argument-clinic topic-tkinter type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @zooba @serhiy-storchaka @larryhastings