1. Victor Stinner
  2. hachoir
  3. Pull requests

Pull requests

#1 Declined
Repository
oliver_g
Branch
prs-pak
Repository
haypo
Branch
default

new parser, for PAK archive files from Project: Starfighter game

Author
  1. oliver_g
Reviewers
Description
No description
  • Learn about pull requests

Comments (4)

  1. Robert Xiao

    I see that you want to add a testcase file...but testcases are downloaded from haypo's server right now.

    I think we should move the testcase files themselves into the repository, if they aren't too big. That way it would be easier to add new testcases in the future.

    Aside from that, the proposed changes look good. I wonder if it might be prudent to add some extra validation checks since PACK might not be a unique magic number. Maybe e.g. checking if there is a first file with a sane size (unless it is legal to have empty PAK files?)

  2. oliver_g author

    Good point about the magic bytes; I've made validate() more strict now. A valid empty PAK file would just have the four bytes "PACK"... I guess such a generic file should not be handled by this parser :-)

    The test file is available at http://www.mail-archive.com/hachoir@lists.tuxfamily.org/msg00535.html . I agree it might be better to have the test files in a repo; but adding them to the hachoir repository would increase its size by 11 MB. Maybe they could be stored in a separate repository on Bitbucket, and the hachoir-parser/tests/download_testcase.py script could be changed to do a hg clone?