You invited me to supply fixes to the README issues I mentioned, and I can do that, but there are a couple of code issues that IMO need addressing, so I need to get a read on your intentions.
It looks like I'd be providing a patch that modifies your patch, is that what's expected?
You're welcome to just modify this one and re-submit it and I'll apply the fixed-up one. If you'd rather (for attribution or other reasons) patch this patch, then that's fine too.
I think the default value (no directive in chirp.config) is rather strange and I'm guessing not what was intended? if addrfmt is None: addrfmt = '%(block)03i' results in e.g.
000: 41 48 30 31 37 24 0b 00 AH017$.. 001: 01 c1 37 73 21 00 00 00 ..7s!... 002: 0e 00 0f 00 0c 00 00 00 ........ -003: 12 02 00 00 00 01 00 01 ........ +003: 12 02 00 00 00 00 00 01 ........ -004: 00 01 00 00 18 06 1d 2e ........ +004: 00 01 00 00 01 06 1d 2e ........ 005: 00 01 00 03 03 00 01 00 ........
addrfmt = '%(addr)03i'would be equivalent to the previous behavior, and much more likely to be useful for most users.
Yes, I was concentrating on getting things plumbed to demonstrate the functionality. I intended for it to remain the same by default.
As I said in an earlier mail, you can generate OverflowError with %(addr)c and ValueError for %(addr)Q. With this code, also TypeError with %(addrfmt)i, and KeyError with %(foobar)i.
In an earlier sketch of this approach, you suggested catching expected exceptions, but that's not in this code.
Yep, catching those sounds good to me.
%(data)s took about 4 minutes to print an empty window and logged an error message, but didn't crash or actually really hang, so that's probably fine.
It's a developer thing, so I think it's probably fine, yeah.
Shall I fix these, and the README file?
Please, thanks.
--Dan