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

Refactor creation of puz files and add optional PUZv2 support #204

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

afontenot
Copy link
Contributor

Most of the downloaders follow a similar pattern of creating a puzzle file with puz.Puzzle() in their parse_xword method and returning this puzzle file to the controller. This refactor changes this to make puzzle files attributes of the BaseDownloader class, which the downloaders then use as needed.

This has several benefits:

  • The Puzzle interface is now abstract, from the point of view of the downloaders. None of them imports puz any more. This enables a potential future PR to allow swapping out puz for a compatible implementation, e.g. one that would create files in a different format like .ipuz (libipuz).

  • As part of this PR, we add a flag -2 --v2 that creates PUZ v2.0 files, rather than the 1.x version. Not all software supports these files, and this is not enabled by default, but for software that does (Gnome Crosswords is an example), we don't have to strip out e.g. emojis, which are becoming increasingly common in online crosswords.

Most of the downloaders follow a similar pattern of creating a
puzzle file with `puz.Puzzle()` in their `parse_xword` method
and returning this puzzle file to the controller. This refactor
changes this to make puzzle files attributes of the
`BaseDownloader` class, which the downloaders then use as needed.

This has several benefits:

 * The `Puzzle` interface is now abstract, from the point of view
   of the downloaders. None of them imports `puz` any more. This
   enables a potential future PR to allow swapping out `puz` for
   a compatible implementation, e.g. one that would create files
   in a different format like .ipuz (libipuz).

 * As part of this PR, we add a flag `-2` `--v2` that creates PUZ
   v2.0 files, rather than the 1.x version. Not all software
   supports these files, and this is not enabled by default, but
   for software that does (Gnome Crosswords is an example), we
   don't have to strip out e.g. emojis, which are becoming
   increasingly common in online crosswords.
@afontenot
Copy link
Contributor Author

There are a couple of things that need looking at, e.g. I don't really understand what's up with the rebus table encoding or whether that is impacted by this PR.

There are at least two other ways you could do this refactor. If you'd prefer one of these, I'm happy to redo it that way.

  • Have the controller make a Puzzle and send it to parse_xword rather than having it as a long-lived attribute of BaseDownloader.
  • Make our own abstract Puzzle class in a separate file, which could import and use puz.Puzzle or some other library (possibly, in the future). Individual downloaders would import from this file rather than puz directly, but would continue to use the resulting Puzzle object as they do now.

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.

1 participant