search

eagleflo / mpyq

Python library for reading MPQ archives.
BSD 2-Clause "Simplified" License
100 stars 23 forks source link

When reporting invalid files, show the invalid magic. #22

Closed GraylinKim closed 11 years ago

GraylinKim commented 11 years ago

This came up when I was doing python3 support. When you know the file you put in should be valid you want to see what the magic came up as and why it was wrong.

eagleflo commented 11 years ago

Hmm. While this is a fairly small change I'm not entirely convinced that it makes sense to report the faulty magic prefix to users. What will they do with the information?

If the magic prefix is misaligned or missing altogether, reading the archive will not be possible. An enterprising user will immediately reach for their hexdump tool of choice and figure out why the header isn't correct. Misalignments are easy to see (like demonstrated in #19), and if it's missing altogether I fear the random garbage in its place won't help the user in any way.

Could you describe your use case a little bit more? It sounds like debugging rather than normal usage.

GraylinKim commented 11 years ago

Yeah, I was just debugging. We can throw this out as not so useful to people.

I used it specifically debugging a python3 issue where the magic (now bytes) didn't match the strings. Very narrow case.

GraylinKim commented 11 years ago

It just seemed like a small thing that might be occasionally useful. If it'll sow more fear and confusion than it is worth then I've got not problem with us throwing this commit out.

eagleflo commented 11 years ago

I'm not going to merge this for now. The error message seems informative enough. If the person encountering this error is familiar enough with MPQ to know about the magic headers, he can probably use a hex editor as well.

Thanks for the pull request anyway!