Skip to content

Add edge case to remove duplicate help arguments#288

Closed
Mark-McCracken wants to merge 1 commit into
apple:mainfrom
Mark-McCracken:issue/259
Closed

Add edge case to remove duplicate help arguments#288
Mark-McCracken wants to merge 1 commit into
apple:mainfrom
Mark-McCracken:issue/259

Conversation

@Mark-McCracken
Copy link
Copy Markdown

Resolves #259

Literally ignores any --help flag, if the first argument is help.

I did spend a while digging into the behaviour using the debugger, but this code is fairly complex, and adding an edge case at this low level would make it harder to understand I think. This ultimately is a little hacky, but I think better than making that low level complexity more complex.

FYI, it was happening somewhere in this function:

var parsedCommand = try parseCurrent(&split)

If you add a breakpoint on the next line, run the program, continue program execution 2 times, then the firstUnused property jumps from 1 to 3, skipping the unused commands, causing our bug.

Checklist

  • I've added at least one test that validates that my change is working, if appropriate
  • [questionable X] I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • [not necessary] I've updated the documentation if necessary

@natecook1000
Copy link
Copy Markdown
Member

Thanks for taking this on, @Mark-McCracken! We need a bit of a different approach than what you've taken here, since commands are free to define different names for the --help flag (using the helpNames parameter on CommandConfiguration) and not every command has a help subcommand.

Instead, I think we could look at making changes to the HelpCommand type, which is an internal command type that generates help for the selected subcommand. When someone executes a command like math help stats average, HelpCommand is the one that gets matched and created, with "stats" and "average" as its arguments, and it figures out which command it should display help for from there.

@Mark-McCracken
Copy link
Copy Markdown
Author

Thanks so much for the feedback!
I'll take another look and try your way instead, and will be back in a few days no doubt

@natecook1000
Copy link
Copy Markdown
Member

@Mark-McCracken I resolved this issue in #309 — it ended up being a little more involved than I expected. Thanks again for looking at tackling this, and please let me know if I can help with any other issues you'd like to take on!

@Mark-McCracken
Copy link
Copy Markdown
Author

Hi Nate, thanks so much, really sorry I didn't get any further with it, I did dig into it a bit, but found that the HelpCommand was actually getting a different input in the first place, and had a bit of a hard time with debugging it.
I did struggle a little and then I had some work commitments I had to spend my free time on. Thanks for sorting it!

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.

Ignore --help flag when specified on a cmd help subcommand command

2 participants