Optional arguments

[jpal91]

Which issue does this fix?

Closes #5

Hello! I discovered this package recently, and I'm a big fan! I was hoping that optional arguments were possible, but saw that there was an issue referencing that hadn't been touched in a while.

I'm not sure if this project is in active development anymore so I decided to implement it based on what I saw in the issue and share it with the community if you're still interested!

Describe the solution

mask-parser

This was the bulk of the change. I altered the parse_command_name... function in parser.rs to look for both parenthesis and brackets. It uses similar logic as before (splitting the string) and essentially just does it twice. It now returns the new struct OptionalArg along with the previous return values.

Additional Changes:

• Added OptionalArg which is identical to RequiredArg
  • This could theoretically be an enum instead of two separate structs, but I decided to keep it as is in case their logic diverged anymore (ie optional with default values)
• Small change to the parse function to accept new values from aforementioned function
• Expanded test in parser.rs to include new field optional_args

mask

• Went back to most every function referencing required_args and mirrored an additional loop for optional_args
  • Most of the loops are identical, but main::get_command_options unwraps to an empty str instead of panicking, making it optional
• Added additional tests in tests/arguments_and_flags_test and small changes to other tests that needed optional_args added to the json

Other notes

• I had to add a php action to the Github actions. MacOS was failing on php not being in path for some reason?
• The order in which required and optional arguments are defined in the maskfile doesn't matter with this implementation which could lead to some weirdness but may be desirable to some? Regardless, with a little additional logic it could be altered

[jacobdeichert]

Thanks for the PR! I plan on reviewing it sometime within the next few days/week when I find the time.

Mask is definitely still in development! I just rarely find time to work on it and in its current form it solves 99% of my day to day needs. Optional args I've wanted for a long time and it just never became a high priority, so I'm looking forwards to getting this reviewed and merged 🙂

[jacobdeichert]

Hey, I did a very quick review and looked into that CI php error with a suggestion on how to fix it properly.

I also noticed the actions didn't run on this PR so I'm hoping once you commit some updates they'll re-run correctly.

[jacobdeichert]

Thanks for getting up to date with master! I haven't forgot about this, just been really busy lately. I plan on checking this out again this week 👍

[jacobdeichert]

@jpal91 the PR looks great! Just the one comment about the macOS runner needing to be reverted so that CI can pass.

After that, will approve and merge!

[jacobdeichert]

Looks great! I'll merge today and plan on cutting a new release in the next week or so.