Re: [chirp_devel] [developer] Control address format for full-file dump/diff - #1767
Well, I can see why a Python aficionado would like that. 8-)
My first reaction is concern for error handling, it seems like it would be pretty easy to specify something with too few or two many arguments. Yeah, the audience is developers and they can probably sort out the exception, but it seems like buying trouble.
The second thing is that my own preferred format is "both" which sort of breaks your model:
out += "%04i x%04X: " % ((i * line_sz), (i * line_sz))
This requires the statement to have two (identical) address arguments in a list. Handling that would require scanning the supplied format, and while I'm sure it's computable, it's more work than I'm interested in, and this flexibility doesn't get me very excited.
If you're OK with letting the user deal with whatever exception they might cause, I could see handling the case of "not decimal or hex or both" by assuming one address argument and just pasting whatever the user supplies for the format.
It doesn't solve any problem I have, but I think it's minimal work and I could do that. I.e.
Eliminate this:
- elif (addrfmt != "decimal" and addrfmt != "hex" and addrfmt != "both"):
print "Invalid hexdump_addrfmt value %s. Using decimal." % addrfmt
addrfmt = "decimal"
and change the print statements to:
...
elif addrfmt == "both":
out += "%04i x%04X: " % ((i * line_sz), (i * line_sz))
elif addrfmt == "decimal":
out += "%04i: " % (i * line_sz)
else out += addrfmt % (i * line_sz)
Thoughts?
-dan
On Jul 15, 2014, at 5:30 PM, Dan Smith - dsmith@danplanet.com wrote:
try:
addrfmt = CONF.get("hexdump_addrfmt", "developer")
except Exception:
addrfmt = "decimal"
if addrfmt == None:
addrfmt = "decimal"
elif (addrfmt != "decimal" and addrfmt != "hex" and addrfmt != "both"):
print "Invalid hexdump_addrfmt value %s. Using decimal." % addrfmt
addrfmt = "decimal"
for i in range(0, (len(data)/line_sz)):
out += "%03i: " % (i * line_sz)
if addrfmt == "hex":
out += "x%04X: " % (i * line_sz)
elif addrfmt == "both":
out += "%04i x%04X: " % ((i * line_sz), (i * line_sz))
else:
out += "%04i: " % (i * line_sz)
Just a thought...
Instead of having them choose from a few different symbolic choices, why not just let them specify the format themselves? So in the config:
hexdump_addrfmt = %0000x
and in the code:
fmt = CONF.get("hexdump_addrfmt", "developer") . . . thing = fmt % addr
I can just see someone coming along and wanting octal or something and if we just let them specify the format, we can avoid adding new symbols. What do you think?
--Dan
The second thing is that my own preferred format is "both" which sort of breaks your model:
out += "%04i x%04X: " % ((i * line_sz), (i * line_sz))
I think you could do this:
result = fmt % locals()
and then I could make a format like this to get what you want:
"%(line_sz)04i %(line_sz)04X"
Should be less code than what you have now and more flexible I think. Just catch ValueError, TypeError, and KeyError and you should be safe for error handling I think.
--Dan
participants (2)
-
chirp.cordless@xoxy.net
-
Dan Smith