Re: [chirp_devel] [developer] Control fontsize of hex diff/dumpdisplay - #1681
Ok, that's the sort of reception I was hoping for, and I don't mind iterating. My code is mostly imitation of what I find elsewhere in the Chirp code, and I recognize I might be missing some nuances.
Hold integrating the patch, I will resubmit in a day or two.
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?
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.
Thanks!
-dan
On Jun 5, 2014, at 4:15 PM, Dan Smith - dsmith@danplanet.com wrote:
- try:
fontsize = CONF.get_int("diff_fontsize", "developer")
- except Exception:
fontsize = 11
If you're going to add hidden configuration options (which I think is probably reasonable for these sorts of things), lets start a README.developers in the top level directory where we at least mention that they exist.
Also, please don't just catch Exception, here, catch whatever ConfigParser raises if it's not there.
- fontdesc = pango.FontDescription("Courier 11")
- fontdesc = pango.FontDescription("Courier %i" % fontsize)
I think some fonts don't have all the sizes between 1 and infinity. What happens if you specify something unsupported? If it's something we can catch and fall back to 11 again, that'd be good. That said, if it's a hidden developer-only tunable, it's not a huge deal.
But in general, if this makes things easier for you, I'm cool with it.
Thanks!
--Dan
chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers
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
participants (2)
-
chirp.cordless@xoxy.net
-
Dan Smith