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

Using argparse module to parse commandline arguments #5

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

Conversation

DeepSpace2
Copy link

Hi! nice side project :)

I've created this PR to introduce the usage of the built-in argparse module to handle commandline arguments instead of manually parsing sys.argv.

I also fixed a small bug in the current implementation that allowed the usage of --solution flag to create a game with a non-guessable solution (for example, you could create a game using abcde as a solution, but could never guess it),

solution = cli_args.solution
player.warn(f"Solution will be {solution}")

else:

Choose a reason for hiding this comment

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

This is likely out of scope for this PR, but since you're touching this code anyway. What are your thoughts on dropping --today and making it the default and adding --random. Everyone I've shown this project to have all had the same expectation that just starting this app up should give you today's wordle not a random wordle.

This would be an api change of course and would warrant a "major version release" of sorts :)

Copy link
Author

Choose a reason for hiding this comment

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

I definitely agree. I'll make this change.

Choose a reason for hiding this comment

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

Any thoughts on this @klipspringr ?

Copy link
Owner

Choose a reason for hiding this comment

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

I can understand that expectation, and if I started again I'd probably make this the default. Equally, the original audience for this was MetaFilter users who had already played the day's official game online and wanted more.

I think as society has probably reached Peak Wordle, at this point I'll only change existing user-facing API/behaviour if it's actually broken.

@klipspringr
Copy link
Owner

So, I should definitely have been using argparse from the beginning -- this PR is much nicer than my manual effort! Thank you for the contribution!

However this PR changes the usage from --today|DAY|SOLUTION to --today|--day DAY|--solution SOLUTION. I'm not sure I want to make a user-facing change for coding style reasons. I had a quick look at implementing the existing behaviour in argparse but I'm not sure it would be any cleaner than the existing code.

I have gone ahead and fixed (855b58) the bug you reported re allowing unguessable solutions (thanks!). I think it's fine to allow a fixed solution that is not in the official solutions list -- it just has to be in the guesses list.

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

3 participants