-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-86017: Improve font actual docs #22434
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
base: main
Are you sure you want to change the base?
Conversation
Thanks Tal for adding the label! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are excellent improvements to the docs!
I have a few suggestions.
Doc/library/tkinter.font.rst
Outdated
|
||
If *option* is specified, the value of just that attribute is returned; | ||
if it is omitted, the return value is a dictionary of all the attributes | ||
and their values. See above for a list of the possible attributes. [1]_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This says "above" but the reference is to a separate page. Is "above" referencing the constructor's docs? If so, let's add a link to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "above" is referring to "additional keyword options". The link is to give reasonable credit to the man page as the entirety of the actual
docs is just a modified version of the content found there (though I'm open to a more clear way of showing this).
@@ -59,7 +68,7 @@ The different font weights and slants are: | |||
|
|||
.. method:: copy() | |||
|
|||
Return new instance of the current font. | |||
Return new instance of the current font with a different *name*. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Doc/library/tkinter.font.rst
Outdated
https://www.tcl.tk/man/tcl8.6/TkCmd/font.htm#M5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be very useful to many who wouldn't know where to find this otherwise!
Note that |
The Tk docs are released under a permissive license that allows free use, but require attribution. See full quote of the license below. (source: the Would you be able to re-word that part in your own words, so we can avoid having to include the full attribution notice?
|
Oh... I'll work on it! |
Thanks for the rewrite, @E-Paine, looking good! Perhaps let's separate the last sentence about displayof into a new paragraph, and mention what type of object it is: Is it a string? What does it reference? Can we give an example? |
@taleinat did you mean to repeat what you said earlier in the review about separating the last sentence into its own paragraph? I believe this is already resolved but can look at it again if you want. |
Yes, it was intentional. I should have mentioned that I was repeating it. The current text still doesn't explain what "displayof" refers to and what type(s) it supports. |
@taleinat I am not quite sure I understand. I had already said it accepted any tkinter widget but have attempted to clarify in the latest commit. |
Unless someone beats me, I intend to review, fix the merge conflict (easy context change), and likely merge. I opened a draft issue on IDLE Issues under Configure to not forget forever: "Use font.actual to compare user font to defaults". |
Given that Serhiy does not see the reason for a
equal
method and Tal was unaware of theactual
method, I instead propose a major improvement to theactual
docs. The docs are currently very brief and do not offer any explanation on what the kwargs are for. I have completely removed the existing doc and instead added a modified version of the tk man's description.With it being so heavy based on the man page, I assume we need to give credit but would appreciate if someone could double check I have done so correctly.
EDIT:
I do not intend to add a news entry as I deem this a fairly trivial change, though obviously cannot add the skip-news tag myself (so will just ignore the bedevere/news failure).
https://bugs.python.org/issue41851