akamensky / argparse

Argparse for golang. Just because `flag` sucks
MIT License
611 stars 62 forks source link

Custom help #50

Closed densestvoid closed 4 years ago

densestvoid commented 4 years ago

Finishes addressing issue. Removed check function to reduce repetitive code and enable extended help argument handling.

akamensky commented 4 years ago

Also note, that this decreases test coverage which currently fails the merge checks. Some newly added code is not covered by tests.

akamensky commented 4 years ago

@densestvoid

I feel this is rather complex PR that adds unnecessary new methods to the parser (and commands).

How do you feel instead of taking this approach to have a method that simply overrides default behavior.

For example:

p := argparse.NewParser("", "")
p.SetHelpSettings("h", "help", false, true)

where function signature would be something like:

func (...) SetHelpSettings(sname, lname stirng, disabled, noExit bool)

Also this PR shows a decrease in test coverage, which is set to fail merge checks.

densestvoid commented 4 years ago

Seems like a reasonable solution, I'll adapt what I currently have and add code coverage to pass validation.

densestvoid commented 4 years ago

@akamensky Ready for review. I took your suggested SetHelpSettings functions and split it. Please let me know if anything needs changed. If this PR is satisfactory, it would solve issue #29 .

akamensky commented 4 years ago

Looks good. Merged.