Tcl/Tk 8.6 now has a fontchooser command. I have developed a tkinter interface to it, similar to colorchooser/askcolor. How does one go about contributing this or proposing for future enhancement to the tkinter module suite?

You've already done the proposing part by opening this issue ;).  To contribute you work, attach it here as a patch to the tkinter sources found in Lib/tkinter/ of a cpython source checkout.  You'll need to sign a contributor agreement before we can look at or accept it, the tracker should prompt you to do so when you attach your patch.  The devguide is also a great resource for questions you may have while preparing your patch.

Thanks for your interest in contributing!

https://www.tcl.tk/man/tcl/TkCmd/fontchooser.htm
I agree that this should be wrapped, as is colorchooser.  There are a few details that may need to be thrashed out.  It is a nuisance that the native font dialog is modal on Windows and not on Mac (and ??? on *nix?).

Not having any luck creating a patch. I have retrieved source, built python, put my fontchooser.py file (attached) in cpython\Lib\tkinter, done hg add and hg commit, but when I try to do hg diff it gives me nothing.

I would recommend backing out your commit (hg rollback if you haven't pulled or otherwise changed your checkout since you made your commit), and just do 'hg diff' at the point where you would commit.  In this particular case, if there are no changes other than the added file it doesn't really matter much that it's not in patch form.

It would be nice to have some unit tests.  It may not be possible to test anything but your _str_to_font and _font_to_str functions, though.

I notice that your Fontchooser class doesn't inherit from commondialog.Dialog like colorchooser.Chooser does; is commondialog.Dialog usable for this?  Or are there improvements that can be made to commondialog.Dialog to make it suitable for Fontchooser and also improve colorchooser.Chooser?  Can this reuse anything from Lib/tkinter/font.py or perhaps be merged into that file to keep all the font stuff together?

After we have some of those details ironed out, this is going to make a nice addition to tkinter!

Good points, Zachary.

Regarding the commondialog.Dialog class, I think it would need to be enhanced to make sense using it with Fontchooser. It really doesn't do a whole lot, and the awful kludge in the show method (creating a Frame widget just so that some low-level operations can be performed) is repugnant to me. I think the whole thing is pretty out-of-date.

With the Font class, perhaps some of its functionality can be used. I will need to better understand the architecture of Font and see what I can do with it.

I think another issue that needs to be addressed is the modal/non-modal aspect of Tk fontchooser. I have access only to Windows OS, where it is modal. I have read that it is non-modal on Mac and perhaps other platforms. If that is the case, then the show method may not work properly on those other platforms. On Windows, the Tk command "tk fontchooser show" does not return control to the caller until after the user has clicked the OK or Cancel buttons. If it does return immediately on other platforms, then the show method may need do an update loop (or something better?) to keep the behavior consistent on the Python side.

+1 I also think that the fontchooser should be wrapped. I briefly tested your wrapper, Lance, and found it worked fine on Windows but did not on my Linux setup (because it is non-modal).

I went about implementing my own wrapper based loosely around your's (see attached) and the way I overcame this problem was to use vwait (this still works on Windows where the call is modal). vwait is good because it returns once the condition is fulfilled without the need for polling or similar.

While I use the commondialog module in the wrapper, this is simply for its handling (in __init__) of the master (I don't think it would make sense to enhance the Dialog class for this wrapper).

> the awful kludge in the show method (creating a Frame widget just so that some low-level operations can be performed) is repugnant to me

You're going to moan at me then for using something similar in the attached! The reason I use it is to avoid the logic found on line 77 of your wrapper and *hopefully* make the code more readable (whether I achieve this is somewhat debatable).

> I have access only to Windows OS

Is this still the case or can you now test on a different system which is non-modal?


Do you wish to go ahead with authoring a patch for this or would you prefer me to do it? (I would appreciate you reviewing it either way and you would still get credit in the news as I took sections from your wrapper) If everything goes well, we can hopefully get this into Python 3.10 (as this is a new feature, it won't be back-ported to 3.9 and before).

Elaine, I was just having a look at this the other day too! I agree, this is definitely worth some effort to get done.

To be honest, I'm not a fan of the vwait solution to force the dialog to be modal. It doesn't actually make it modal (i.e. you can still do stuff in other windows). Moreover, the Mac version (which is native, so couldn't easily be made modal via Tkinter) has a very different design than on other platforms. It doesn't have "ok", "apply", "cancel" buttons or similar. It's designed to just hang around. 

When the font dialog was originally proposed for Tk (see http://tip.tcl.tk/324) they'd tried a modal API first and found it unworkable.

In light of that, my preference for the official wrapper would be to mirror the underlying API, as per what Lance's version first tried, using a callback mechanism to return results. Most importantly, it would eliminate the assumption that there's always a way to make it modal. Documentation would be key. Mind you, there's no official docs for the other dialogs. I can guarantee it would be well-documented with an example on TkDocs anyway!

If there aren't any strong objections, I'm able to have a go at modifying Lance's version over the next week or so. I've got Mac, Windows, and Linux environments to test everything out. 

Question... I assume the official wrapper needs checks to ensure fontchooser is available (i.e. 8.6 or greater). If not, should the wrapper just error out? Would providing an alternative implementation for Tk < 8.6 be something that would be done in an external module, or might that be something that could/should be included in the official library?

For future reference, if anyone is wondering why the font chooser is so complicated to use in a way that makes sense across platforms, here is its current behaviour...

From the manual:
 - configure -font is the font currently shown or font shown when dialog is initially shown (neither guaranteed on all platforms)
 - implementation-dependent which actions result in callback being called and event being sent

Windows
 - modal; show blocks until dismissed, cannot interact with other windows
 - ok/cancel
 - apply button added if a command option is specified
 - with command (apply button present)
     - if apply: callback generated with font currently set in dialog, event generated [configure -font is NOT updated]
     - if ok: callback generated with font in dialog, dialog closed, event generated, configure -font not updated
 - if no command (no apply button)
     - on ok, get event, configure -font not updated (correct, since dialog not visible)
 - fontchange event not generated if option is set in code

X11
 - not modal; show returns immediately, can interact with other windows
 - ok/cancel
 - apply button added if a command option is specified
 - with command (apply button present):
     - if apply: callback generated with font currently set in dialog, event generated [configure -font is NOT updated)
     - if ok: callback generated with font in dialog, dialog closed; no event, configure -font NOT updated
 - with no command (no apply button):
     - no event generated, configure -font NOT updated
 - fontchnaged event generated if option is set in code, configure -font updated
 - configure -font never updated by user interaction
 - conclusion: need to set command, hold onto current value returned

macOS
 - no ok/cancel buttons, works like a palette
 - non-modal; show returns immediately, can interact with other windows
 - BUG:can appear when tk first loaded (sometimes..)
    - happens when left open on previous launches and program exited abnormally e.g. ctl-C in terminal
    - ~/Library/Saved Application State/com.tcltk.wish.savedState/windows.plist still holds font chooser
    - if so, -visible initially is false, but is true after idle... no intervening <<TkFontChooserVisibility>> event
    - will segfault if set options e.g. font
    - workaround: hide on startup
 - fontchange event generated on every change in dialog, configure -font updated to font in dialog
 - fontchange event generated when option set in code, configure -font updated to font in dialog
 - command callback (if specified) invoked on every change from user (not in code), configure -font updated

I've put together the first cut of a wrapper that tries to smooth over some of the non-essential differences in implementation details across platforms, while still respecting essential platform conventions. It also works around a few bugs I discovered along the way.

It includes the wrapper class itself, a minimal demo, a more realistic demo, and the notes about current behaviour of the underlying Tk widget on different platforms.

Current snapshot is at https://github.com/roseman/tkdocs/blob/fontchooser/fontchooser.py

Obviously, this borrows hugely from previous snapshots by Lance and Elisha. Thanks!

If you get a chance to try it out, would appreciate feedback if this is on the right track or not.

I have started looking at Mark's wrapper (though I have not been able to
​spend as much time on it as I would like). It's mostly small changes
​that I have made (in no particular order):
- TkFontchooserFontChanged event calls 'command' (to be behave like an
'apply' button - this only occurs on platforms that set font due to user
​actions, otherwise we don't know what font has been selected)
- a cget method (which getitem now forwards to)
- 'show' takes 'configure' kwargs
- allow instantiation without kwargs before root creation*
- 'ismodal' decorated as property
- 'compatible' module attribute set at import (`tkinter.TkVersion >= 8.6`)
- winfo_toplevel used to ensure parent is a window (see comment at bottom)
- TkFontchooserVisibility has a visibilitycommand kwarg**

* In most use cases, the user will only want a single instance of the
Chooser class through the entire application run and then just change
change the parent as required. The idea here is that we can instantiate
the class at import and then lazy-load the parent so the user can call
something like `tkinter.fontchooser.chooser.show()`. This is the reason
​I decided to decorate 'ismodal' as a property, rather than having it as
​a variable set on class load.

** I would prefer to keep the user from binding virtual events to the
parent themselves and instead intend to provide a kwarg for a command to
be called when this event is generated (I think binding to the parent is
needlessly complicated, especially when we call winfo_toplevel before
​passing to Tk anyway, so I thought it was either this or we provide the
​user with a 'bind' method for the class). Between this proposal and
​TkFontchooserFontChanged calling 'command', this would eliminate all
need for the user to bind the events themselves (hence, they should not
​be included in the docs).

I haven't yet properly tested it (just focusing on changing the
​user-facing api), however I think what Mark has got so far is very close
​to what we should submit as a patch (my ideas are attached as a diff of
​Mark's file). The above are just QOL changes to *hopefully* make it
​easier for the user, rather than another rewrite of the wrapper.

> Documentation would be key

Definitely! I think this is quite possibly the hardest part of the patch
(after deciding what is the 'cleanest' api for the user) because we need
​to ensure the docs are detailed and precise while being accessible to
​those who don't know Tk (and hence won't be digging through the Tk man -
​in which case, as helpful as the "Notable differences from underlying Tk
​API" is internally, it should not be part of the docs).

> I assume the official wrapper needs checks to ensure fontchooser is
> available (i.e. 8.6 or greater). If not, should the wrapper just error out?

Personally, I would just leave the wrapper without any checks for
version validity. All the official installers use 8.6 and there are very
​few distros that don't provide 8.6 as their default Tk package (most
​notably CentOS 6 & 7). We should provide a module-level attribute that
​checks the Tk version for the user (this is the 'compatible' attribute
​included in the diff) but I don't think we should do anything beyond
​this (it is hence up to the user to ensure they are on a compatible
​system). Applications like IDLE, which try to be compatible with older
​versions of Tk, should provide their own fallback interface rather than
​building such an interface into tkinter.


I am not the biggest fan of the current implementation of ismodal as
​this is based purely on Mark's testing of just 3 windowing systems
​(Windows, X11 & MacOS - my point is about the limited number not that I
​doubt Mark's testing!). While I think the BSDs also use X11, we are
​effectively declaring that there are no other windowing systems (on an
​OS CPython supports) where fontchooser is modal (which I very reluctant
​to do). At the same time, though, I don't have a clue how else it would
​be implemented in a user-friendly way (I can think of a couple of ways
​but all of them involve showing the fontchooser temporarily).

My suggestion is, therefore, to raise a NotImplementedError if the
​current windowing system is not one of 'win32', 'x11' or 'aqua'
​(because, of these, we know that only win32 is modal - the docs would
​have to note this limited availability).


Addressing a couple of Mark's points in an email he sent me:

> the -parent option doesn’t need to be a toplevel
In my limited testing (on X11), I found that the virtual events were not
​called unless this was true (or at least it appeared so...).

> there’s some argument to be made that because the bind lets you
​> trigger multiple callbacks it may promote looser coupling
While I see the point, I think the same could be said of Tk having
​-command rather than a virtual binding. I also think we should pick one
​of either kwargs or virtual events and then stick to that to avoid an
​inconsistent api (in my diff, I chose the former of the two).