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

Added keyword arguments to Font.render #2000

Merged
merged 21 commits into from
May 17, 2023

Conversation

PurityLake
Copy link
Contributor

Refers to #1990

Added keyword argument options for the arguments in Font.render

@PurityLake PurityLake requested a review from a team as a code owner March 8, 2023 01:53
@PurityLake
Copy link
Contributor Author

Oddly enough, despite the docs saying wraplength is an arg to Font.render it was never passed on to the C implementation.

Also the C implementation didn't receive the antialias argument.

@Starbuck5
Copy link
Member

That’s not what ftfont is.
Ftfont is “font” on the freetype API.

@PurityLake
Copy link
Contributor Author

Ah apologies, will rectify that mistake

docs/reST/ref/font.rst Outdated Show resolved Hide resolved
src_c/font.c Outdated Show resolved Hide resolved
@Starbuck5 Starbuck5 added the font pygame.font label Mar 10, 2023
Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

test/font_test.py Outdated Show resolved Hide resolved
dr0id
dr0id previously requested changes Mar 18, 2023
Copy link
Contributor

@dr0id dr0id left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about the parameter names I find color and background more understandable than fgcolor and bgcolor. Reading this I would wonder what fgcolor is.

Maybe textcolor and backgroundcolor would be nicer (but longer) and would need a change in Font and in Freetype.

docs/reST/ref/font.rst Outdated Show resolved Hide resolved
docs/reST/ref/font.rst Outdated Show resolved Hide resolved
@Starbuck5
Copy link
Member

Maybe textcolor and backgroundcolor would be nicer (but longer) and would need a change in Font and in Freetype.

We can't change freetype since it already has those as kwargs. What about color and bgcolor ?

@MyreMylar
Copy link
Member

MyreMylar commented Mar 19, 2023

Maybe textcolor and backgroundcolor would be nicer (but longer) and would need a change in Font and in Freetype.

We can't change freetype since it already has those as kwargs. What about color and bgcolor ?

color and bgcolor would match the names used in classic HTML tags (the <body> and <font> tags to be precise).

@Starbuck5
Copy link
Member

@PurityLake This looks to be nearly done, just needs a bit of sprucing up.

It seems we've come to a consensus on the kwarg names being color and bgcolor, so that needs to be updated in the docs/types/code/test.

The versionchanged needs to be updated to 2.3.0, since that's the next version now.

After those changes I think this is good to go.

test/font_test.py Outdated Show resolved Hide resolved
@Starbuck5 Starbuck5 dismissed dr0id’s stale review May 17, 2023 03:20

OP fixed the issue

@Starbuck5 Starbuck5 added this to the 2.3 milestone May 17, 2023
@Starbuck5 Starbuck5 merged commit 2f93a16 into pygame-community:main May 17, 2023
29 checks passed
@PurityLake PurityLake deleted the kwargs-font-render branch May 17, 2023 04:36
Mega-JC pushed a commit to Mega-JC/pygame-ce that referenced this pull request May 21, 2023
* Added kwargs to font.render

* Added docs and tests for kwargs for font.render

* Fix incompatible pointer

* Added wraplength to font.render

* Attempt to fix failing test

* Remove changes to ftfont.py

* Change a test to only run for FontTest class

* Fix formatting issue

* Adjusted keyword arg names for font.render and updated docs

* Fix documentation error

* Changed arguments fgcolor and bgcolor in ftfont

* Adjusted tests to include freetype font

* Adjusted formatting

* Fixed failing test in font_test

* Fixing font_test again

* Fixing font_test yet again

* Fix formatting

* Renamed fgcolor to color

* Fixed remaining mentions of background
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
font pygame.font
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants