Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix picotool to work with p8 carts that use CRLF line endings #100

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

scottnm
Copy link

@scottnm scottnm commented Apr 11, 2022

picotool fails to read p8 carts which use CRLF line endings

this generally isn't a huge issue but git and windows make for really annoying problems dealing with line endings and given pico8 supports carts with CRLF line endings, it seems like a nice convenience for picotool to support both as well

this change...

  1. Fixes picotool to support CRLF line endings
  2. Fixes picotool to provide better error messages when a file fails to parse due to invalid headers
  3. Updates the gitattributes so that git won't try to normalize the line endings of the test p8 cart files in the repo
  4. Adds a new test for verifying that p8 carts with CRLF line endings are parsed correctly
  5. Adds an automated test when merging to master to validate that the tests all pass on both Windows and Ubuntu

1. Fix the header parsing and GFX section parsing to not fail when lines end in CRLF
2. Clarify the invalid header failures between version failures and other types
3. add a specific test for CRLF carts
@scottnm scottnm marked this pull request as ready for review April 11, 2022 05:29
@@ -1,5 +1,7 @@
# picotool: Tools and Python libraries for manipulating PICO-8 game files

[![tests](https://github.com/scottnm/picotool/actions/workflows/python-app.yml/badge.svg)](https://github.com/scottnm/picotool/actions/workflows/python-app.yml)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops. I probably need to fix this to point to this repo's test button

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant