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

update system font lists in sysfont.py #545

Merged
merged 7 commits into from Oct 16, 2018
Merged

update system font lists in sysfont.py #545

merged 7 commits into from Oct 16, 2018

Conversation

@tripping-alien
Copy link
Contributor

@tripping-alien tripping-alien commented Oct 2, 2018

mac os fix for issue: #179

i've done this today in a couple of hours and didn't notice that there is another pull request for this issue.

@tripping-alien tripping-alien reopened this Oct 2, 2018
@tripping-alien
Copy link
Contributor Author

@tripping-alien tripping-alien commented Oct 2, 2018

fixed some problems in code, seems to be working good now.

Copy link

@e1000 e1000 left a comment

dunno which PR is better, just I like to give pythonic coding style suggestions :)

src_py/sysfont.py Outdated Show resolved Hide resolved
src_py/sysfont.py Show resolved Hide resolved
lines_len = len(lines);

try:
for idx in range(0, lines_len):

This comment has been minimized.

@e1000

e1000 Oct 2, 2018

no need of variable lines_len and idx , just loop over lines :

for line in lines:
  key, value = line.strip().split(':', 1)
  ...

also , if you do the lower() here :

for line in lines:
  key, value = line.strip().lower().split(':', 1)
  ...

then, you can remove all the other calls to lower() which would more clean imho ...

This comment has been minimized.

@tripping-alien

tripping-alien Oct 2, 2018
Author Contributor

can't use lower() on the whole line because mac's file system is case sensitive and this line can contain a path to the font file, i fixed it for key

@illume illume mentioned this pull request Oct 16, 2018
4 tasks done
@illume illume changed the base branch from master to andreyxfont Oct 16, 2018
@illume illume merged commit fe8267b into pygame:andreyxfont Oct 16, 2018
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants