Custom Font Family Support

[jrbostic]

It looks like font_family property is secretly supported as a configuration attribute. It almost works completely, however the author field on the title page appears to be rendered with the wrong encoding somehow (so it comes out with garbled characters). This would be a really useful configuration for a potentially small investment.

[jrbostic]

I guess this includes both the 'Credit' and 'Author' properties on the title page.

[jrbostic]

Oh, I'm somewhat mistaken here. It appears that font_family will default to CourierPrime, but if provided an alternative value, it will use Courier and fail to properly render those two properties?

[ifrost]

Hi. Thanks for the feedback. CourierPrime is the default font_family. If used it's embeded in PDF. Any other family_font should fallback to system Courier font. Support for custom fonts has been on my list for long time now (#25 #36) but it hasn't been high priority as CourierPrime and Courier work well in most cases.

[jrbostic]

Oh sorry, I didn't realize those tickets were related to the same issue. Is there any kind roadmap or landmarks for this update? Any specific files or locations that you know to be involved in making the improvement? If you're willing to accept pull requests, I could look into it... I'm just almost entirely ignorant of what the work would involve

[ifrost]

New fonts can be added to https://github.com/ifrost/afterwriting-labs/blob/master/js/utils/fonts.js. Courier Prime is encoded with base64 here https://github.com/ifrost/afterwriting-labs/blob/master/js/utils/courier-prime-font.js. When a custom font name is provided it's embeded to PDF here: https://github.com/ifrost/afterwriting-labs/blob/master/js/utils/pdfmaker.js#L63. To make new fonts available from web interface it also has to be added to settings https://github.com/ifrost/afterwriting-labs/blob/master/js/plugin/settings/model/settings-config-provider.js#L30.

Sorry but there's not much documentation so you will need to play with it. If you want to use it only for yourself you can simply try to to replace Courier Prime with different font. PRs are welcome, but in this case there's one problem. At the moment most of the codebase is shared between web version and command line interface. I don't want to increase the build size with new fonts as it'd slow down loading web version. To make it work nicely it'd require dynamic font loading or extracting font loading so it's easy to have different set of fonts in CLI.

To be honest recently I haven't had much time to work on 'afterwriting. I hope to get back to regular updates in a few weeks.

[jrbostic]

Alright, I've got a prototype going. I'd like to clean it up and have you take a look, to see if the design is acceptable to you and/or what you can't live with (before I go and dump time into testing or documenting). Basically, it allows for passing an optional JSON file reference into the command line that is then used to add to the font list. The JSON file mimics your internal font map structure and it would support even lazily passing in multiple font profiles via a single JSON object, while the font_family setting still determines which font is actually used. This keeps the fonts entirely out of your build but would allow CLI users to specify any font profiles they'd like. I also want to move the base64 decoding out into a file util, rather than in the PrimeCourier file. That's basically the whole story. I'm presently working on the cleanup and a separate integration branch for an Atom IDE plugin that uses the CLI, so do let me know if any of that sounds just plain wrong. Thanks!

[jrbostic]

I missed a semicolon, and I'm having some odd trouble getting a proper diff against develop... but this is what I was tentatively thinking...

https://github.com/ifrost/afterwriting-labs/pull/94

[ifrost]

I like it! I'll check it in the afternoon but it's looking good. I don't use develop but rather master + feature branches model (described a bit more in https://github.com/ifrost/afterwriting-labs/blob/master/docs/process.md). Please add the missing semicolon to pass quality check.

What fonts are you going to use? There are some font properties that are not linked with the font but rather hardcoded, like font_width, font_height, max characters per line in https://github.com/ifrost/afterwriting-labs/blob/master/js/utils/print-profiles.js. These numbers are set up to work correctly with Courier and assume font is monospaced to format PDF correctly. font_width is used for proper spit dialogue, font_height to correctly lay out title page so please check how it looks with the fonts you're gonna use. The best way is to use sample script containing different fountain markup - https://github.com/ifrost/afterwriting-labs/blob/master/test/data/screenplays/test.fountain.

[jrbostic]

Thanks for the info. For now, I'm just looking to support some alternative monospaced fonts. And see if I can't explain the 'Author' and 'Credit' properties' problem rendering. I'm adding those semicolons now and seeing what I can do in the way of testing/documenting.

[jrbostic]

Okay, I've updated to fix the code quality check and add docs. I had trouble finding a CLI test to go off of. Could you advise on what you'd like to see in that department? Is there a section in the tests that directly hits the portions of code I modified?

[jrbostic]

Regarding the 'Author' and 'Credit' rendering in default Courier (the image above)... I'm starting to wonder if maybe one of the associated font files in this project isn't corrupt. When I pass in custom fonts, they appear to render fine.

[jrbostic]

@ifrost Sorry to call you by name, but can you give me an idea of what kind of testing you'd like to see here? Anything else you'd like to see or that's problematic? I'm willing to do whatever it takes to get this in... when you get time, any additional feedback would be appreciated.

[ifrost]

I've merged your PR and published new version to npm (1.10.0). Thanks for contributing!

There should be a unit test for PDFController but because the change is not affecting browser app I'm happy to merge it. CLI is a bit second-class citizen at the moment - test coverage is mostly based on acceptance tests, but they are written only for the browser app.

Thanks!

[jrbostic]

Hey, much appreciated! I'll swing back around and try to add a test for that controller.