Add support for loading TrueType font files. by MarronEyes · Pull Request #1960

Add support for loading TrueType font files. by MarronEyes · Pull Request #1960 · pygments/pygments

In the ImageFormatter class, we can load ttf fonts that already exists in the operating system.
However, the "font_name" argument does not accept a custom font (a filename or a file-like object containing a ttf font).
I hope that this pull request solves this problem.

I think this is a good feature to have, only two questions:

how to handle different font styles? At the moment this is impossible.
I think it's more idiomatic to check for hasattr(obj, 'read') to check for file-like objects, no?

We can maybe ask the user to provide different styles of the font only if he wants to use a custom font.
You are right. I will change it.

Thanks! Could you please update the documentation of the parameter (in the docstring of ImageFormatter)?

…e user specified a variable font in "font_name" argument.

Sorry for the long inactivity, I wasn't available at the moment.
I added a get_style function to get the specified style of the font in get_font.
The code is tested and it works! The pr can be merged.

This also looks good to me, but there's no test for it (nor do I see how this can be tested easily.) @birkenfeld , @jean-abou-samra , any concerns with that? I'm not super happy to merge code without tests but I don't see an easy way to test image based output.

		return font
		except ValueError:
		pass
		except OSError:

Choose a reason for hiding this comment

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

This indicates an invalid font, right? Shouldn't there be a warning or an error?

		font.set_variation_by_name(style_name)
		return font
		except ValueError:
		pass

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Does it guarantee that no ValueError is raised? I'd guess some of the code could do that ... alternative would be to catch only OSError and pass in all other cases?

		bold and italic fonts will be generated. This really should be a
		monospace font to look sane.
		If a filename or a file-like object is specified, the user must
		provide different styles of the font.

Choose a reason for hiding this comment

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

If I understand correctly, a typo in the file name will result in font_name being understood as the name of a font and not a path, and the errors will read 'No usable fonts named ...' or something like that depending on the system. Can we make it explicit rather than implicit and use a separate font_path parameter?

I'm not super happy to merge code without tests but I don't see an easy way to test image based output.

No, I don't see one either, especially that font stuff is so platform-dependent :( So I think it's OK if this doesn't have tests.

Merged for 2.16. Let's see if this works in the wild, thanks for the contribution!