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

[docs] Improve documentation for fontTools.ttLib.ttFont #2442

Merged
merged 7 commits into from
Nov 18, 2021

Conversation

simoncozens
Copy link
Collaborator

  • Allow cross-references in docs
  • Point users more clearly to ttFont
  • Remove old example
  • Typos
  • Improve documentation for ttFont module

@chrissimpkins
Copy link
Member

chrissimpkins commented Nov 8, 2021

It looks like the RTD configuration now supports builds on PR's. Helpful to review doc updates in a Sphinx rendered format?

@simoncozens
Copy link
Collaborator Author

simoncozens commented Nov 10, 2021

I'd strongly recommend whoever has admin access to RTD (@anthrotype?) to enable this. ( https://docs.readthedocs.io/en/stable/pull-requests.html ) My next documentation PR is going to be significant.

@anthrotype
Copy link
Member

I just enabled that, hope it works. Thanks Simon, I'll take a look soon

@belluzj
Copy link
Contributor

belluzj commented Nov 10, 2021

@anthrotype I'm interested in the RTD deploy on PR thing, how do I enable it on the designspace PR #2436?

@anthrotype
Copy link
Member

Honestly I have no idea, I simply followed the above linked instructions to enable the feature. I never used RTD myself

@chrissimpkins
Copy link
Member

chrissimpkins commented Nov 10, 2021

Do you have admin access on the repo in RTD Jany? If you do I can show you where to flip the switch. It will show up as a new build line item in the GH CI with a link to the PR specific doc build on RTD. If you activate it on an existing PR it should show up with the next branch commit.

@anthrotype
Copy link
Member

As I said, I have already flipped that switch. Maybe it'll only show up on new PRs

@chrissimpkins
Copy link
Member

chrissimpkins commented Nov 10, 2021

apologies, read this on my phone and was under the assumption that this was for a different repo.

@simoncozens
Copy link
Collaborator Author

It doesn’t seem to be working - didn’t show up on #2446 which was created after the config change.

@chrissimpkins
Copy link
Member

Webhook does not seem to be working for some reason. There must be a RTD configuration issue on the fonttools settings side. You might need to approve the RTD OAuth app.

@anthrotype I am getting this information on https://readthedocs.org/dashboard/fonttools/integrations/118984/

The project fonttools doesn't have a valid webhook set up, commits won't trigger new builds for this project. See the project integrations for more information.

and I don't have access to modify webhooks here.

@anthrotype
Copy link
Member

I went to that page and clicked Resync Webhook, and now I no longer seeing that message about not having a valid hook set up. Please try again and let me know if it is working now. Also I think a user named "simpkins" is already an admin and co-maintainer on the RTD fontools account.

@chrissimpkins
Copy link
Member

chrissimpkins commented Nov 11, 2021

Yes I have admin access on the RTD account but not on the fonttools repo. It wouldn't let me re-sync the webhook. I think Cosimo's change should fix this. Hopefully this works for you too @belluzj


Example interactive session:

Python 1.5.2c1 (#43, Mar 9 1999, 13:06:43) [CW PPC w/GUSI w/MSL]
Copy link
Member

Choose a reason for hiding this comment

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

Python 1.5.2c1, March 9 1999... 🥲

Args:
file: Similarly to the constructor, can be either a pathname or a writable
file object.
reorderTables (bool): If true (the default), reorder the tables, sorting
Copy link
Member

Choose a reason for hiding this comment

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

reorderTables is an Optional[bool]. When it's set to None (e.g. the subsetter does that), it means write tables in whatever order is most convenient (mostly due to inter-table dependencies at compile time) without caring about either the canonical order or the original font order. This should be the fastest mode

@@ -320,11 +337,15 @@ def importXML(self, fileOrPath, quiet=None):
reader.read()

def isLoaded(self, tag):
"""Return true if the table identified by 'tag' has been
"""Return true if the table identified by ``tag`` has been
decompiled and loaded into memory."""
return tag in self.tables

def has_key(self, tag):
Copy link
Member

Choose a reason for hiding this comment

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

we should deprecate has_key in favour of __contains__ at some point

@simoncozens simoncozens merged commit e0dc30a into fonttools:main Nov 18, 2021
@simoncozens simoncozens deleted the ttfont-docs branch November 18, 2021 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants