Well, not quite what I expected. I have tried font sizes of 400, 300000, 0, -12, and -1234 and Pango does not generate an exception, it fact is seems to render what was asked for, mostly.

0 was a blank screen. 300000 actually seemed to be realized; scrolling about the window I was able to find some glyph edges. Of course, it's completely unusable, but there's no exception to handle. Doing some reading about Pango on the web, I think this is what it's built to do.

The negative values were readable, but variable-width, making me nervous that something got mangled. But no exception generated.

That's on a Mac, so Windows or linux might behave differently, but there's no exception I can generate and thus handle.

Then out of whimsy I tried "twelve" (no quotes), expecting that would generate an exception from CONF.get_int(). Nope. It returned 0, no exception, resulting in an empty window. I suspect that's related to

However, I see that the ChirpConfigProxy wrapper is not going to raise a sane exception for the case you're going to hit here.

So between the odd behavior for negative fontsizes, and the lack of exception for non-integer, I've added some bounds checking, accepting [4, 144]. The values were chosen as marginally absurd, or more specifically, exceeding anything I expect is useful, but rendering recognizably so the developer-user can see they got what they asked for but probably not what they want. And minimal chance that someone will request larger/smaller down the line, even with 4k displays.

Resubmitted patch to follow shortly. If you still want refinements I remain open to that, but I can't see what else to do at the moment.

-dan

On Jun 6, 2014, at 3:39 PM, Dan Smith - dsmith@danplanet.com wrote:

...

For the first, what you're suggesting would be except: fontsize = 11 ?? I did test what happens if diff_fontsize is not there in chirp.config and what I submitted handles it. Does that cover your concern?

Well, you can ignore this for the moment. In general, it's bad form to catch Exception, which will swallow anything. It's better to catch the actual thing you're expecting, so that if anything unexpected happens, it bubbles up and gets noticed.

However, I see that the ChirpConfigProxy wrapper is not going to raise a sane exception for the case you're going to hit here. So, I'll retract my comment on the grounds that my code you're calling is kinda broken :)

...

I'll look at specifying some weird fontsize and see if I can get an exception generated and make sure it's handled. I thought about bounds checking, but I figured the users are developers and could deal with the results of specifying something strange. But if it's just a try/except clause, seems worth it.

Well, just see what happens. If it's something we can just log to the debug log that the size was unsupported and default back to 11, that seems like a good idea.

--Dan